Skip to content

Add dashboard font selection - #487

Merged
m-aebrer merged 9 commits into
aebrer:masterfrom
maxscheurer:feature/issue-486-dashboard-font-selection
Aug 28, 2026
Merged

Add dashboard font selection#487
m-aebrer merged 9 commits into
aebrer:masterfrom
maxscheurer:feature/issue-486-dashboard-font-selection

Conversation

@maxscheurer

Copy link
Copy Markdown
Contributor

Closes #486

Add browser-local dashboard font selection while preserving existing theme-default typography and color-mode behavior.

Implementation plan posted as a comment below.

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Implementation Plan

Problem analysis

The dashboard already routes its main typography through --mono-font, but font choice is coupled to theme: Gruvbox supplies JetBrains Mono and every other theme supplies IBM Plex Mono. The browser-local appearance lifecycle already solves the harder surrounding concerns—validated persistence, clean defaults, DOM reflection, first-paint bootstrap, cross-tab synchronization, and resilient fallback when browser APIs fail—so font selection should extend that lifecycle rather than introduce server or RPC state.

The CSS cascade needs deliberate handling. Theme rules set --mono-font on both <html> and locally scoped gallery cards, while .theme-card-code is hard-pinned to IBM Plex Mono to prevent an inactive Gruvbox card from loading JetBrains Mono. An explicit font override must outrank theme font declarations on the document, produce intentional preview behavior, and preserve alternate-font request isolation.

One existing typography escape is directly relevant: .memory-textarea uses the undefined --font-mono token instead of --mono-font. Correcting it is required so an explicit dashboard font applies consistently to the memory editor rather than only to inherited body/code surfaces.

Resolved product and technical choices

  • Offer the two families the dashboard already supports: IBM Plex Mono and JetBrains Mono. Do not add new font assets or dependencies in this change.
  • Include Theme default as the first/default option. With that option selected, preserve existing behavior exactly: Gruvbox uses JetBrains Mono; all other themes use IBM Plex Mono.
  • Persist explicit choices per browser in localStorage, using the existing appearance module’s validation, default-value key removal, storage-event synchronization, and in-memory fallback patterns. No server, RPC, settings-file, or TUI-theme integration.
  • Represent only explicit overrides on <html>; omit the font attribute and storage key for Theme default. A final font-override CSS mapping will set --mono-font after theme declarations so explicit choice wins independently of theme.
  • Add a compact labeled font <select> beside the existing appearance controls, ordered Theme default, IBM Plex Mono, JetBrains Mono.
  • Theme cards will reflect an explicit font selection. In Theme default mode, previews retain the existing baseline IBM rendering so opening Settings does not fetch JetBrains Mono solely for the inactive Gruvbox card.
  • Keep the existing hosting model: IBM Plex Mono remains loaded through the existing Google Fonts stylesheet; JetBrains Mono remains the existing self-hosted, OFL-licensed, lazily used asset.
  • Extend the synchronous head bootstrap so a persisted explicit font attribute is present before stylesheets evaluate, avoiding an attribute-level wrong-font flash. Normal font-display: swap behavior remains unchanged while a webfont loads.

Deliverables

1. Font preference lifecycle

  • Add a fixed font preference catalog/type with stable IDs and labels for Theme default, IBM Plex Mono, and JetBrains Mono.
  • Add validation, browser-local storage, a Solid accessor/setter, default cleanup, and DOM reflection through a data-font attribute for explicit choices.
  • Include font state in reload, initialization, storage-event ownership, storage.clear() reconciliation, and test teardown/reset.
  • Preserve the existing theme/color-mode signals, attributes, color-scheme, and live theme-color behavior without coupling them to font selection.

2. First-paint restoration

  • Extend the pre-stylesheet index.html bootstrap with the font storage key and explicit-font allowlist.
  • Apply only valid, non-default font values; invalid or retired values leave no font attribute.
  • Extend the bootstrap contract tests so storage keys and font IDs cannot drift between the inline script and the TypeScript catalog.

3. Settings control and previews

  • Add a labeled font selector to the existing ThemeGallery appearance control surface, with browser-local and Theme-default behavior explained in concise hint text.
  • Apply changes immediately and keep the control available independently of server-backed settings success.
  • Reflect explicit font choices in theme preview cards while retaining the current no-eager-JetBrains behavior in Theme default mode.
  • Update the surrounding Settings appearance copy and footnote to name theme, light/dark mode, and font as per-browser dashboard settings independent of the TUI.

4. Typography cascade and loading behavior

  • Add final CSS mappings from explicit font IDs to complete fallback stacks through --mono-font, ordered so the explicit selection overrides every theme’s default font.
  • Adjust preview styling so explicit selection is visible without making Theme default eagerly request JetBrains Mono.
  • Correct .memory-textarea to consume --mono-font, ensuring the selected family applies to that editor as well as body, code, preformatted text, and inherited controls.
  • Update stale source comments that describe typography as permanently theme-owned or the appearance system as only theme plus color mode.
  • Keep the current @font-face, WOFF2 assets, Google Fonts link, font weights/styles, package allowlist, and dependency graph unchanged.

5. Documentation

Document the three-option font behavior, per-browser persistence, clean Theme-default semantics, theme independence, preview behavior, and existing remote/self-hosted loading model consistently across all public surfaces:

  • Root dashboard capability summary.
  • Coding-agent README dashboard capability summary.
  • Dashboard guide Settings and Appearance system sections.
  • Dashboard package README feature and architecture sections.
  • JetBrains Mono provenance note, replacing the now-incomplete statement that the asset is used only when Gruvbox is active.

Files to modify

  • packages/dashboard/src/client/state/appearance.ts — font catalog/type, validation, signal, storage, setter, DOM attribute, listener/reload/reset integration, and corrected module comments.
  • packages/dashboard/src/client/index.html — pre-stylesheet font restoration and updated bootstrap documentation.
  • packages/dashboard/src/client/components/theme-gallery.tsx — font selector and preview font-state reflection.
  • packages/dashboard/src/client/screens/settings.tsx — appearance/footnote copy that includes font selection.
  • packages/dashboard/src/client/styles/themes.css — explicit font-to-token mappings and updated font-loading comments.
  • packages/dashboard/src/client/styles/app.css — preview behavior and the --font-mono to --mono-font memory-editor correction.
  • packages/dashboard/src/client/styles/tokens.css — baseline typography comments, without changing the pristine token value.
  • packages/dashboard/test/client/appearance.test.ts — font catalog, validation, persistence, resilience, DOM, synchronization, reset, and bootstrap contracts.
  • packages/dashboard/test/client/screens.test.tsx — control rendering/options, restoration, interaction, clean reset, independence from theme, preview attributes, and settings copy.
  • packages/dashboard/test/client/appearance.browser.test.ts — computed-family precedence, first-paint restoration, representative typography consumers, preview behavior, and font-request isolation.
  • README.md — public dashboard appearance summary.
  • packages/coding-agent/README.md — dashboard capability summary.
  • packages/coding-agent/docs/dashboard.md — Settings and Appearance system documentation.
  • packages/dashboard/README.md — dashboard feature/architecture/loading documentation.
  • packages/dashboard/src/client/assets/fonts/PROVENANCE.md — accurate JetBrains Mono activation conditions.

No new files, font binaries, packages, protocol fields, or server modules are planned.

Acceptance criteria

  • Settings offers Theme default, IBM Plex Mono, and JetBrains Mono in that order, and selecting any option applies immediately.
  • Theme default stores no font key, sets no data-font attribute, and preserves current typography: Gruvbox resolves JetBrains Mono; every other theme resolves IBM Plex Mono.
  • Explicit IBM Plex Mono overrides Gruvbox; explicit JetBrains Mono overrides every non-Gruvbox theme. Theme and color-mode changes do not clear or replace the explicit font choice.
  • A valid explicit selection persists per browser, restores before stylesheets evaluate, and synchronizes across open tabs. Invalid/stale values and unavailable storage fail safely to Theme default.
  • The selected family reaches body text, code/preformatted text, inherited controls, and the memory editor through --mono-font.
  • Theme preview cards show an explicit selected family. In Theme default mode, opening Settings alone does not fetch JetBrains Mono for the inactive Gruvbox preview.
  • JetBrains Mono is requested when it is the active theme default or an explicit override, and is not requested when IBM Plex Mono explicitly overrides Gruvbox.
  • Existing theme palettes, mode/OS precedence, color-scheme handling, live theme-color metadata, scoped preview colors, and pristine default behavior remain unchanged.
  • Dashboard font settings remain browser-local and independent of server settings, RPC, TUI themes, and the host settings file.
  • All five documentation/provenance surfaces describe the shipped behavior consistently.

Testing approach

Unit and jsdom coverage — appearance.test.ts

  • Pin font catalog order, unique IDs, labels, and validator behavior.
  • Cover missing, valid, invalid, and retired persisted values.
  • Verify setter updates signal/storage/data-font, Theme default removes the key/attribute, and invalid setter input normalizes safely.
  • Extend storage read/write failure tests so the in-memory font signal remains authoritative.
  • Verify font-key storage events reload state, unrelated events remain ignored, and null-key clear resets theme/mode/font together.
  • Confirm repeated initialization still registers listeners once and reset tears font state down.
  • Extend index.html storage-key and allowlist contracts with every explicit font ID while excluding Theme default.

Component coverage — screens.test.tsx

  • Assert the font selector and exact option order render in the resilient dashboard-local section.
  • Restore an explicit stored font and verify selected control/root state.
  • Change between both explicit families and Theme default; assert immediate signal, storage, and DOM effects.
  • Change theme/mode after choosing a font and assert the font preference remains independent.
  • Assert each preview card carries the expected explicit preview font state, and Theme default keeps the lazy baseline behavior.
  • Update and pin user-facing per-browser/TUI-independence copy.

Real-browser coverage — appearance.browser.test.ts

  • Extend computed-font tests across representative theme/font combinations: automatic default, automatic Gruvbox, IBM override on Gruvbox, and JetBrains override on a non-Gruvbox theme.
  • Observe network requests and document.fonts to verify JetBrains Mono loads only when actually used, including explicit selection and Settings-preview scenarios.
  • Verify the real head bootstrap applies a seeded font before the unavailable app module can run and ignores invalid values.
  • Assert scoped theme colors remain independent while preview typography follows the defined explicit/default behavior.
  • Probe representative body/code/control/memory-editor elements to ensure the final computed family consistently follows --mono-font.

Verification

  1. Format/lint touched files with Biome.
  2. Run focused dashboard appearance, settings-screen, and real-browser suites.
  3. Run npm run build before any manual test against the real dashboard binary.
  4. Run npm run verify-workspace-links.
  5. Run the full npm test suite under Node 22.
  6. Manually verify all three font options with Gruvbox and a non-Gruvbox theme on desktop and narrow/mobile Settings layouts, including the memory editor and font loading in browser network tools.

Risks and mitigations

  • Theme/font specificity conflicts: place explicit font mappings in the final appearance layer and test opposing theme/override combinations through computed styles.
  • Scoped preview inheritance: give cards deliberate preview font state rather than relying accidentally on root inheritance; test palette isolation and font behavior separately.
  • Eager alternate-font fetches: preserve a baseline preview in Theme default and use real network observation rather than inferring request behavior from CSS strings.
  • Wrong-font first frame: restore validated explicit font state in the synchronous head bootstrap and pin script-before-stylesheet ordering.
  • Default/migration regression: make Theme default the no-key/no-attribute case, so every existing user keeps the exact current theme-owned mapping.
  • Storage drift: include the new key in owned-event filtering, null-key reconciliation, reset helpers, and inline-bootstrap contract tests.
  • Layout shifts and glyph coverage: reuse only the already-supported families/assets and manually inspect representative dense, code, status, and editor surfaces.
  • Documentation drift: update all public dashboard summaries plus the asset provenance note in the same change.

Open questions

No blocking questions remain for the bounded first cut. Additional font families, self-hosting IBM Plex Mono, a dedicated visual font gallery, or account/server synchronization are additive follow-ups and are intentionally outside this plan.


Plan created by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Progress Update

Implemented browser-local dashboard font selection across state, first-paint bootstrap, UI, CSS, tests, and documentation.

  • Added Theme default, IBM Plex Mono, and JetBrains Mono options to Settings. Theme default preserves the existing theme-owned mapping; explicit choices override any theme.
  • Added validated localStorage persistence, clean default key/attribute removal, data-font DOM reflection, storage-event synchronization, safe fallback behavior, and synchronous pre-stylesheet restoration.
  • Made explicit fonts visible in theme previews while keeping Theme default previews on IBM Plex Mono so opening Settings does not eagerly fetch JetBrains Mono.
  • Preserved the existing Google-hosted IBM and self-hosted/lazy JetBrains asset model.
  • Corrected the memory editor's undefined font token so it follows --mono-font with the rest of the dashboard.
  • Updated all public dashboard documentation plus JetBrains Mono provenance.

Verification: Biome clean · build passes · 377 focused dashboard tests pass · full monorepo suite passes (5,969 tests) · pre-commit suite passes (5,891 tests, 717 skipped) · workspace links valid · manual Playwright verification passed at desktop and narrow viewports against the built dashboard.

Commit: d4437c5


Progress tracked by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

would be cool to also include a bundled Dyslexia-friendly font!

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Implementation Plan Addendum — Bundled OpenDyslexic

Suggested addition

Add OpenDyslexic as a fourth font-picker option after JetBrains Mono. It is the clearest match for the maintainer request: the family is explicitly designed around common dyslexia symptoms, is available under SIL OFL 1.1, and ships regular, bold, italic, and bold-italic WOFF2 faces.

Use the unmodified upstream WOFF2 files from the official antijingoist/opendyslexic repository’s v0.91.12 tag, recording the tag, commit, source URLs, checksums, copyright, and Reserved Font Name in provenance. Keeping the binaries unmodified avoids Reserved Font Name concerns that would arise from subsetting while still allowing fully local, lazy browser loading.

Documentation should describe OpenDyslexic as designed for readers with dyslexia, not claim guaranteed medical or readability outcomes.

Deliverables

1. Catalog and first-paint support

  • Extend FontId, FONTS, and FONT_IDS with stable id opendyslexic and label OpenDyslexic.
  • Add the same id to the synchronous index.html font allowlist so persisted selection restores before stylesheets evaluate.
  • Reuse the existing generic validation, persistence, data-font, cross-tab, Theme-default cleanup, and preview-state behavior; no new lifecycle or server state is needed.

2. Bundled font faces

Add four unmodified, pinned upstream assets under packages/dashboard/src/client/assets/fonts/:

  • opendyslexic-regular.woff2
  • opendyslexic-italic.woff2
  • opendyslexic-bold.woff2
  • opendyslexic-bold-italic.woff2

Declare matching @font-face rules in themes.css for normal/italic and regular/bold typography. Map the existing dashboard weights 500–800 to the upstream bold face so headings and emphasized controls use a real bundled face rather than synthesized bold. Add the final override:

  • data-font="opendyslexic""OpenDyslexic", "IBM Plex Mono", "Courier New", monospace

Do not preload the family. Vite should fingerprint and emit the four WOFF2 files only through the CSS references, and browsers should request them only after OpenDyslexic becomes active.

3. License, provenance, and publication

  • Add a distinct verbatim OPENDYSLEXIC-OFL.txt; do not reuse or overwrite the existing JetBrains Mono license file.
  • Add OPENDYSLEXIC-PROVENANCE.md with the pinned tag/commit, exact upstream files, checksums, face mapping, and Reserved Font Name note.
  • Extend packages/dashboard/package.json’s files allowlist with the OpenDyslexic license and provenance files. The WOFF2 binaries continue to ship through built dist/ assets.
  • Verify the built CSS URLs and npm pack --dry-run contents so all four fingerprinted faces and both legal/provenance files are present in the published package.

4. UI and documentation

  • The existing catalog-driven <select> and preview-card logic should render and apply OpenDyslexic without bespoke component behavior.
  • Update the root README, coding-agent README, dashboard guide, dashboard package README, and font-loading/provenance wording to list OpenDyslexic as a bundled, browser-local explicit option.
  • Keep Theme default semantics unchanged: most themes remain IBM Plex Mono, Gruvbox remains JetBrains Mono, and OpenDyslexic is used only when explicitly selected.

Files to add

  • packages/dashboard/src/client/assets/fonts/opendyslexic-regular.woff2
  • packages/dashboard/src/client/assets/fonts/opendyslexic-italic.woff2
  • packages/dashboard/src/client/assets/fonts/opendyslexic-bold.woff2
  • packages/dashboard/src/client/assets/fonts/opendyslexic-bold-italic.woff2
  • packages/dashboard/src/client/assets/fonts/OPENDYSLEXIC-OFL.txt
  • packages/dashboard/src/client/assets/fonts/OPENDYSLEXIC-PROVENANCE.md

Files to modify

  • packages/dashboard/src/client/state/appearance.ts — catalog/type only.
  • packages/dashboard/src/client/index.html — bootstrap font allowlist only.
  • packages/dashboard/src/client/styles/themes.css — four faces, OpenDyslexic token mapping, and loading comments.
  • packages/dashboard/package.json — publish license/provenance files.
  • packages/dashboard/test/client/appearance.test.ts — catalog and bootstrap contracts.
  • packages/dashboard/test/client/screens.test.tsx — option order and reactive preview attributes.
  • packages/dashboard/test/client/appearance.browser.test.ts — local asset harness, computed family, face/glyph coverage, and request isolation.
  • README.md, packages/coding-agent/README.md, packages/coding-agent/docs/dashboard.md, packages/dashboard/README.md — public behavior and loading documentation.

No server, RPC, TUI, settings-file, dependency, or theme-default changes are needed.

Acceptance criteria

  • The font selector lists Theme default, IBM Plex Mono, JetBrains Mono, and OpenDyslexic in that order.
  • Selecting OpenDyslexic applies immediately to the dashboard and preview cards, persists per browser, restores before first paint, and remains independent of theme/color mode.
  • Theme default remains the no-key/no-attribute case and retains the current IBM/Gruvbox mapping.
  • Opening Settings in Theme default does not request any OpenDyslexic asset.
  • Selecting OpenDyslexic requests only the locally served, Vite-fingerprinted faces actually used; no CDN request is introduced.
  • Normal, bold, italic, and bold-italic rendered probes resolve to OpenDyslexic without faux-style fallback, and required dashboard glyphs (● ◆ ○ ✕ plus representative coding punctuation) are available.
  • Invalid/stale stored font values still fall back safely to Theme default.
  • The npm package contains all four built WOFF2 assets plus the verbatim OpenDyslexic OFL and provenance record.
  • Public docs accurately distinguish Google-hosted IBM Plex Mono from bundled JetBrains Mono and OpenDyslexic.

Testing approach

  • Unit/jsdom: extend exact catalog ordering, valid IDs, setter/persistence cases, and the inline bootstrap allowlist contract.
  • Settings component: assert the fourth option, selection/restoration, root data-font, and all preview cards’ reactive data-font="opendyslexic" state.
  • Real browser: copy all four assets into the test origin; assert computed OpenDyslexic on body/code/control/memory-editor probes; exercise 400, bold, italic, and bold-italic faces; check required glyph availability through document.fonts; observe no request before selection and local requests after selection.
  • Build/package: run the full build, inspect fingerprinted CSS URLs, and use npm pack --dry-run to confirm binaries/license/provenance.
  • Regression: rerun focused appearance/screens/browser suites, workspace-link verification, and the full monorepo test suite; manually inspect desktop and narrow Settings layouts because OpenDyslexic’s proportional metrics differ from the existing monospace families.

Risks and mitigations

  • Package size: the four unmodified upstream faces add roughly 460 KiB before package compression. Lazy loading prevents a runtime transfer unless selected; document the package-size tradeoff rather than subsetting under the Reserved Font Name.
  • Proportional metrics: OpenDyslexic is not monospace, so code alignment and dense controls will change when users opt in. Test representative code/editor surfaces and treat this as an explicit accessibility preference, not a new default.
  • Weight mapping: the upstream family provides regular and bold rather than every numeric dashboard weight. Map the dashboard’s emphasized range to the real bold face and verify computed font matching in Chromium.
  • Accessibility claims: describe the font’s design intent without promising universal benefit; user choice remains authoritative.
  • License publication: retain a family-specific verbatim OFL and provenance file in the npm tarball, verified before merge.

Plan addendum created for the maintainer-requested bundled dyslexia-friendly font

@maxscheurer
maxscheurer marked this pull request as ready for review August 28, 2026 08:56
@maxscheurer

Copy link
Copy Markdown
Contributor Author

Unverified Review Candidates — Pending Assessment

Review round: 1
Reviewed commit: d4437c5

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Scope note: the authoritative scope includes the original issue plus the maintainer-approved addendum (m-aebrer comment 2026-08-25T17:15:33Z + mach6-plan addendum 2026-08-26T13:33:24Z). The reviewed commit predates both; the addendum is not implemented.

Critical

1. Approved OpenDyslexic scope expansion is entirely unimplemented (confidence 99)
The maintainer-requested bundled dyslexia-friendly font (OpenDyslexic) is missing end-to-end at this head:

  • FontId/FONTS/FONT_IDS (packages/dashboard/src/client/state/appearance.ts:38,64-71) and the settings selector pin exactly 3 options; the addendum requires Theme default, IBM Plex Mono, JetBrains Mono, OpenDyslexic in that order.
  • No opendyslexic-{regular,italic,bold,bold-italic}.woff2 assets under packages/dashboard/src/client/assets/fonts/; no @font-face rules or 500–800→bold weight mapping; no data-font="opendyslexic" mapping in themes.css (only the two explicit mappings at lines 1007–1012).
  • index.html bootstrap allowlist is ["ibm-plex-mono", "jetbrains-mono"]; a stored opendyslexic value would be silently rejected as invalid.
  • No OPENDYSLEXIC-OFL.txt / OPENDYSLEXIC-PROVENANCE.md; packages/dashboard/package.json files lacks the license/provenance entries.
  • Repo-wide grep for "opendyslexic|dyslexic|dyslexia": zero matches — no docs, no tests.
    Fix: implement the addendum as a follow-up commit (catalog + bootstrap allowlist + assets + @font-face + mapping + legal/provenance files + package allowlist + tests + docs), or explicitly re-scope with the maintainer on the PR.

Important

2. Theme-default font matrix lacks computed-font assertions for consumers beyond <pre> (confidence 85)
packages/dashboard/test/client/appearance.browser.test.ts:407 (the "theme-default font requests JetBrains Mono only for gruvbox" matrix) reads computed fontFamily only for #mono. The plan's "representative consumers body/code/controls/memory-editor" is satisfied only in the explicit-font describe. A concrete regression would sail through: hardcoding body or .memory-textarea to the IBM stack in theme-default mode — the gruvbox row would still see JetBrains on #mono and the request check would still pass. Fix: extend the theme-default it.each to also assert computed families for #body-text, #control, and #memory (gruvbox → JetBrains; others → IBM).

Suggestions

3. The .memory-textarea var(--font-mono)var(--mono-font) fix is a pixel no-op, and current assertions cannot falsify it (confidence 85)
--font-mono was defined nowhere (verified against the base commit); an undefined var() in font-family computes to the inherited value, which was already body's var(--mono-font). The fix is correct hygiene and should be kept, but reverting the app.css hunk would leave every existing #memory assertion passing. Optional: assert against a parent with a deliberately different font-family to make the probe meaningful.

4. Color-mode select has no accessible label; the new font row shows the correct pattern right below it (confidence 90)
packages/dashboard/src/client/components/theme-gallery.tsx:53-59#pref-color-mode uses <span class="setting-label">, so the control has no accessible name. The new font row (line 69) correctly uses <label for="pref-font">. One-line fix: make the color-mode row a <label for="pref-color-mode"> too.

5. theme-gallery.tsx file-header doc omits the new data-font scoping attribute (confidence 90)
Lines 3–8 describe the card contract as data-theme + data-color-mode only; the newly introduced data-font (which the inline button comment at lines 86–91 documents) is absent from the header's scoping description.

6. Google-hosted IBM Plex Mono stylesheet is render-blocking; unreachable CDN stalls first paint (confidence 90)
packages/dashboard/src/client/index.html:20-25 — synchronous external <link rel="stylesheet"> in <head>. If fonts.googleapis.com is unreachable (offline, dropped-connection firewall, DNS/regional block), no dashboard content paints until the connection times out. Pre-existing since the theme PR, but this PR makes IBM Plex Mono an explicit user-selectable font, so CDN reliability is now part of the font-selection contract the docs describe — while the other selectable font (JetBrains) is same-origin, lazy, and immune. Options: self-host the IBM WOFF2 (also OFL), load the stylesheet non-blocking, or at minimum media="print" onload="this.media='all'".

7. "Explicit font on a pristine install must not activate color-scheme" is pinned by no test (confidence 80)
applyAppearance and the bootstrap both deliberately exclude the font from the active/color-scheme computation, but no test exercises the font-only state: unit setter tests toggle theme/mode too, and first-paint tests seed font alongside theme+mode (or an invalid font only). A future change adding the font to active in exactly one of the two places would diverge silently. Fix: one unit test (pristine state, setFont("jetbrains-mono"), assert color-scheme empty + data-font set).

8. Bootstrap script recomputes the same indexOf allowlist checks twice (confidence 90)
packages/dashboard/src/client/index.html:49-52THEMES.indexOf(t) !== -1 and MODES.indexOf(m) !== -1 are each evaluated twice. Hoist to hasTheme/hasMode/hasFont booleans so each allowlist check appears once (the PR's third line is where the duplication stopped scaling).

9. JetBrains-woff2 request-capture boilerplate is copy-pasted three times (confidence 85)
packages/dashboard/test/client/appearance.browser.test.ts:422-425, 465-468, 494-497 — the identical fontRequests + page.on("request") block appears three times (one pre-existing, two added by this PR). A trackJetBrainsRequests(page) helper would fit the file's existing shared-helper convention.

10. Cross-tab storage guard grows a negation clause per dimension (confidence 80)
packages/dashboard/src/client/state/appearance.ts:268-277 — four conjunctive event.key !== … negations. An APPEARANCE_STORAGE_KEYS array + includes reads as "not one of our keys" and scales to a fourth dimension. Judgment call: the chain is still readable.

Strengths

  • The font dimension is a surgical, pattern-faithful extension of the appearance module — guarded storage, validators, default-removal, cross-tab sync, reset, and bootstrap all extended uniformly with zero drift from the theme/mode idioms.
  • Cascade design is correct: explicit [data-font] mappings appended after every theme rule win at equal specificity on both <html> and scoped preview cards (statically verified across all --mono-font declarations); no higher-specificity or !important rule can defeat an explicit choice.
  • Preview-card font scoping is a genuinely nice design: one extra attribute preserves the no-eager-JetBrains invariant, covered at both DOM level (all 8 cards) and real-network level (zero woff2 requests in theme-default mode; request fires on explicit JetBrains).
  • Three-layer test depth (unit/jsdom, component, real-Chromium network observation), strict bootstrap contract tests pinning the inline allowlists, invalid-persisted-value rejection covered pre-paint, and Solid reactivity of data-font={previewFont()} verified by compiling the component.
  • All five doc/provenance surfaces updated consistently for the shipped 3-font behavior with no over-claims; the latent var(--font-mono) landmine in .memory-textarea was found and corrected.
  • No scope creep against the original plan: 15 files modified, zero new files, zero server/RPC/TUI/dependency changes.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Review Assessment

#487 (comment)

Independent verification at HEAD d4437c5 (clean tree, working tree == reviewed commit): the reviewed commit is dated 2026-08-25T14:45:24Z and strictly predates the maintainer's OpenDyslexic request (17:15:33Z) and the mach6-plan addendum (2026-08-26T13:33:24Z); the branch has no later commit, and a repo-wide grep for opendyslexic|dyslexic|dyslexia returns zero matches. All ten original plan acceptance criteria were re-verified against actual code and tests and hold at HEAD.

Classifications

Finding Classification Reasoning
1. OpenDyslexic scope expansion unimplemented merge blocker Factual: verified absent at every cited location — FontId/FONTS/FONT_IDS pin 3 ids (appearance.ts:38,64-71), bootstrap allowlist is the 2-element explicit array (index.html:45), assets/fonts/ holds only JetBrains assets, no @font-face/data-font="opendyslexic" rules (themes.css:1007-1013), no OPENDYSLEXIC-OFL/PROVENANCE files, package.json files lacks entries, no docs/tests. Scope: part 3 of the authoritative scope is the verbatim maintainer request plus the approved addendum, which specifies the fourth option end-to-end. Practical: shipping as-is violates four explicit addendum acceptance criteria (four-option selector order, local-only face requests, npm package contents, three-way docs distinction); a user promised the bundled dyslexia-friendly font gets a feature that does not exist.
2. Theme-default matrix asserts computed family for #mono only deferred Factual: partially accurate — the theme-default matrix (appearance.browser.test.ts:407-445) reads only #mono; body/controls/memory are asserted only in the explicit-font tests. Scope: no acceptance criterion mandates per-element theme-default assertions. Practical: the claimed "hardcode-to-IBM in theme-default mode" regression is caught anyway: hardcoding gruvbox's default fails the gruvbox matrix row (request + family), and hardcoding body/controls/memory fails the explicit-font tests that read all four probes. The only uncaught shape (an element-specific font-family change active only when data-font is absent) is contrived. Missing test alone is not a blocker.
3. .memory-textarea var swap is a pixel no-op nitpick Factual: verified — --font-mono is defined nowhere; the old declaration computed to the inherited value (body's var(--mono-font)), so old and new values are pixel-identical. Scope: not required by any criterion. Practical: no user impact either way; the change is harmless defensive hygiene and should be kept, but no fix or extra test is blocking.
4. #pref-color-mode has no accessible label nitpick Factual: verified — the color-mode row uses <span class="setting-label"> (theme-gallery.tsx:50-59). Scope: pre-existing (unchanged context line); no criterion covers a11y labeling; the PR's own added row uses the correct <label for> pattern. Practical: the PR does not introduce or worsen the gap; converting the row to a label is a sensible optional follow-up.
5. theme-gallery.tsx header doc omits data-font nitpick Factual: verified — the header contract paragraph (lines 3-12) mentions only data-theme/data-color-mode; the inline card comment does cover data-font. Scope: internal comment only; public docs unaffected. Practical: no user-visible or behavioral effect.
6. Render-blocking Google Fonts stylesheet deferred Factual: mostly accurate but misattributed — git log -S shows the <link> came from the dashboard foundation PR (#321), not the theme PR; this PR does not touch it and the network contract is byte-identical in every font state. Scope: the plan explicitly accepts Google-hosted IBM as the design (all five doc surfaces describe it). Practical: an offline/firewalled user experiences the same pre-existing delay and the page still renders via the Courier New, monospace fallback; the font feature works either way. Legitimate follow-up (self-host or non-blocking load), not a blocker here.
7. Font-only state → no color-scheme, unpinned by test deferred Factual: verified — font is deliberately excluded from active in both applyAppearance (appearance.ts:181) and the bootstrap (index.html:59), and no test exercises the font-only state. Scope: the plan requires color-scheme behavior unchanged, which HEAD satisfies; no criterion requires the test. Practical: the divergence it would catch is hypothetical and the consequence (retinted native controls for a font-only change) is cosmetic. Behavior at HEAD is correct; useful follow-up test.
8. Bootstrap recomputes indexOf twice nitpick Factual: verified (index.html:56-59). Two extra indexOf calls on tiny arrays per page load; negligible, no scope requirement. Micro-style preference.
9. JetBrains request-capture boilerplate ×3 nitpick Factual: verified — identical capture block at appearance.browser.test.ts ~417, ~465, ~497. Short, correct, low hazard; a helper would fit the file's convention. Style preference.
10. Four conjunctive negations in cross-tab guard nitpick Factual: verified (appearance.ts:269-276). Logic is correct and covered by tests; array+includes is a readability preference with no behavioral difference.

Action Plan

  1. Implement the approved OpenDyslexic addendum on this branch (finding 1), per the 2026-08-26 plan addendum:
    • Extend FontId/FONTS/FONT_IDS with opendyslexic ("OpenDyslexic"), added after JetBrains Mono; add the id to the index.html bootstrap allowlist (and extend the exact-array contract test).
    • Add the four unmodified pinned upstream WOFF2 assets (opendyslexic-{regular,italic,bold,bold-italic}.woff2, antijingoist/opendyslexic v0.91.12) under packages/dashboard/src/client/assets/fonts/, plus @font-face rules in themes.css mapping dashboard weights 500–800 to the bold face and the final data-font="opendyslexic""OpenDyslexic", "IBM Plex Mono", "Courier New", monospace override. No preloading.
    • Add the distinct verbatim OPENDYSLEXIC-OFL.txt and OPENDYSLEXIC-PROVENANCE.md (pinned tag/commit, source URLs, checksums, face mapping, Reserved Font Name note); extend packages/dashboard/package.json files; verify npm pack --dry-run contains all four fingerprinted faces plus both files.
    • Update all five doc/provenance surfaces to distinguish Google-hosted IBM Plex Mono from bundled JetBrains Mono and bundled OpenDyslexic.
    • Extend the tests per the addendum: catalog order (4 options) + bootstrap contract; settings option order and reactive preview attributes; browser suite with the local-asset harness asserting computed family, all four faces without faux-style, glyph availability (● ◆ ○ ✕ + coding punctuation), request isolation, and no CDN request.

Everything else in the PR — the complete 3-font implementation (all ten original acceptance criteria re-verified at HEAD) — is shippable once the blocker above lands.


Assessment by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Progress Update — finding 1 (approved OpenDyslexic addendum) implemented

The round-1 merge blocker is now resolved at the new head: bundled OpenDyslexic is implemented as the fourth dashboard font option, per the maintainer-approved scope expansion and the mach6-plan addendum.

What landed

  • Assets & legal: 4 unmodified WOFF2 faces (regular, bold, italic, bold-italic) from antijingoist/opendyslexic tag v0.91.12 (commit 449a41b), pinned with SHA-256 checksums; verbatim OPENDYSLEXIC-OFL.txt (Copyright Abbie Gonzalez, Reserved Font Name — subsetting intentionally avoided) plus OPENDYSLEXIC-PROVENANCE.md next to the existing JetBrains provenance.
  • Client: FontId/FONTS/bootstrap allowlist extended (order: Theme default → IBM Plex Mono → JetBrains Mono → OpenDyslexic); four @font-face rules with non-overlapping ranges (regular/italic 100 499, bold 500 900 — upstream Bold is usWeightClass 800, so dashboard weights 500–800 use the real bold face, no faux synthesis); final [data-font="opendyslexic"] override stack OpenDyslexic, "IBM Plex Mono", "Courier New", monospace.
  • Packaging: package.json files entries; npm pack --dry-run confirms all four fingerprinted WOFF2s + both legal files ship in the tarball.
  • Docs: all five public surfaces (root README, coding-agent README, docs/dashboard.md, dashboard README) plus the fonts provenance now distinguish Google-hosted IBM Plex Mono from the bundled JetBrains and OpenDyslexic families.
  • Tests: extended unit (appearance.test.ts), component (screens.test.tsx), and browser (appearance.browser.test.ts) suites. New real-browser coverage: theme-default mode never requests an OpenDyslexic asset (verified via a document.fonts load oracle), explicit selection loads all four local faces with zero CDN requests, preview-card isolation, and first-paint restore of the font-only state — which also pins the deferred "font-only does not activate color-scheme" follow-up.

Glyph ground truth (not assumed): fontTools cmap analysis of the exact shipped files shows OpenDyslexic covers ● ○, all coding punctuation, and the typographic characters; ◆ ✕ ↻ are absent and fall back within the stack. That fallback path is identical to the one today's IBM themes already use, since the Google-served IBM Plex Mono slices lack those shapes too — documented in the provenance.

Verification: Biome clean · npm run build emits all four faces into dist/static/assets/ · full monorepo test suite green · workspace links valid · built-dist smoke test in real Chromium (first-paint data-font/data-theme attributes before app code, built-CSS cascade, lazy loading of exactly the faces in use, no CDN requests for OpenDyslexic).

Second commit (unrelated test hygiene): the pre-existing packages/tui test "should treat raw 0x08 as ctrl+backspace in Windows Terminal" was non-hermetic — it sets WT_SESSION but the Windows-Terminal guard also requires the SSH_* vars to be absent, so it failed on any SSH host. The test now clears those vars (50/50 passing); CI was unaffected since runners have no SSH vars.

The other round-1 findings are unchanged: items 2, 6, 7 remain useful deferred follow-ups; the rest were nitpicks.

Commits: a936f8e (OpenDyslexic addendum) · 1420281 (tui test hermeticity)


Progress tracked by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Unverified Review Candidates — Pending Assessment

Review round: 2
Reviewed commit: 1420281

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Scope note: authoritative scope is issue #486 + the original mach6-plan (3-font scope, 10 acceptance criteria) + the maintainer request ("would be cool to also include a bundled Dyslexia-friendly font!") + the approved mach6-plan addendum (OpenDyslexic as fourth option, 9 acceptance criteria). Round 1 reviewed d4437c5 and carried one merge blocker — the addendum was unimplemented. This head adds two commits (a936f8e OpenDyslexic addendum, 1420281 tui test hermeticity). All five specialists plus the review parent independently re-verified the addendum implementation against actual files, re-downloaded binaries, executed test suites, and reproduced npm pack --dry-run; the blocker is assessed as resolved (details in the assessment). No Critical or new blocker-tier candidates emerged.

Critical

None.

Important

1. toContain on computed fontFamily has a demonstrated false-pass shape in the gruvbox×ibm matrix row (confidence 85)
packages/dashboard/test/client/appearance.browser.test.ts:478-505 (matrix) vs themes.css:519 — Gruvbox's theme stack is "JetBrains Mono", "IBM Plex Mono", "Courier New", monospace, i.e. it contains "IBM Plex Mono" as a fallback. So in the gruvbox × ibm-plex-mono row, expect(family).toContain("IBM Plex Mono") passes even if the explicit [data-font] override lost the cascade (computed family would be the gruvbox stack with IBM in fallback position). At HEAD the row is saved by the network assertion (fontRequests must be [] — a JetBrains-first stack fires a real woff2 fetch), so no regression slips through today, but the family oracle provides false confidence in the assertion meant to prove cascade precedence, and the idiom would be copied into any future matrix row. Fix direction: parse the computed value and assert the first family equals the expected one; keep the network assertion as backstop.

2. The OpenDyslexic glyph-availability assertion is near-tautological and host-dependent (confidence 84)
packages/dashboard/test/client/appearance.browser.test.ts:646-654font-probe.html has no Google Fonts <link>, so "IBM Plex Mono" and "Courier New" are unknown/unloaded in the probe; the fallback codepoints (◆ ✕ — …) resolve through the generic monospace, i.e. the test host's system font. document.fonts.check over the stack returns true whenever the host's default monospace covers those codepoints, regardless of what the OpenDyslexic stack's actual fallback chain does; the assertion cannot attribute which font renders which glyph and could not fail on any realistic host. The in-test comment ("● ○ and the punctuation come from OpenDyslexic itself; ◆ ✕ fall back within the stack") overclaims what is verified. Fix direction: add a second check against 16px "OpenDyslexic" alone for the glyphs the family claims to cover (● ○ + coding punctuation) — that would genuinely fail if a bundled face lacked them.

3. The tui hermeticity fix has no meta-guard against future guard variables (confidence 84)
packages/tui/test/keys.test.ts:304-320 vs packages/tui/src/keys.ts:668 — the guard is Boolean(WT_SESSION) && !SSH_CONNECTION && !SSH_CLIENT && !SSH_TTY and the test now clears exactly those three vars (verified correct and hermetic at HEAD — 50/50 with all three set). But nothing would catch a future guard variable: if && !process.env.SSH_ASKPASS is later added to the guard, the test silently becomes non-hermetic again on SSH hosts and CI will never surface it (runners have no SSH env — the same blind spot that hid this bug). Fix direction: clear all SSH_* vars (prefix-based loop) inside the WT_SESSION scope so the test tracks guard evolution.

Suggestions

4. Font-request-capture boilerplate now ×4 (capture block in 6 places) plus a near-verbatim preview-card test clone (confidence 88)
packages/dashboard/test/client/appearance.browser.test.ts:435-439, 486-490, 512-516, 558-562, 586-591, 660-664 — the identical fontRequests + page.on("request") capture block is now four copies of the JetBrains variant plus two OpenDyslexic ones, and the new OpenDyslexic preview-card test (lines ~658-687) is a line-for-line clone of the JetBrains one with the family name swapped. A captureRequests(page, isMatch) helper next to applyRoot plus one it.each over the two self-hosted families would shrink both. (Round-1 finding 9, extended by the addendum work.)

5. Theme-default font matrix still asserts computed family only for #mono (confidence 90)
packages/dashboard/test/client/appearance.browser.test.ts:428-474 — the "theme-default font requests JetBrains Mono only for gruvbox" matrix forces layout and reads computed fontFamily only for #mono; the new #body-text/#control/#memory/#glyphs probes are asserted only in the explicit-font describes. A regression hardcoding body or .memory-textarea to a literal stack in theme-default mode (active only when data-font is absent) would still pass. (Round-1 finding 2 — still open; unchanged this round.)

6. Render-blocking Google Fonts stylesheet remains (confidence 90)
packages/dashboard/src/client/index.html:20-25 — the synchronous external fonts.googleapis.com <link rel="stylesheet"> is byte-unchanged and pre-dates this PR, but IBM Plex Mono is now an explicit user-selectable font, so CDN reachability is part of the documented font contract while the other two selectable families are same-origin. Self-hosting IBM (OFL) or non-blocking load remains a sensible follow-up. (Round-1 finding 6 — unchanged.)

7. Bootstrap allowlist checks are evaluated twice (confidence 95)
packages/dashboard/src/client/index.html:49-52THEMES.indexOf(t) !== -1 and MODES.indexOf(m) !== -1 are each computed once for the setAttribute and again for active. Hoist to themeOk/modeOk booleans. (Round-1 finding 8 — unchanged.)

8. Cross-tab storage guard grows a negated conjunct per dimension (confidence 90)
packages/dashboard/src/client/state/appearance.ts:268-279 — four conjunctive event.key !== … negations. An APPEARANCE_STORAGE_KEYS array + includes (the file's existing idiom) reads as "not one of our keys" and scales to a fourth dimension. (Round-1 finding 10 — unchanged.)

9. theme-gallery.tsx file-header doc omits the data-font scoping attribute (confidence 95)
packages/dashboard/src/client/components/theme-gallery.tsx:3-14 — the header contract paragraph describes cards as carrying data-theme + data-color-mode only; data-font is documented only in the inline button comment (lines 86-90). (Round-1 finding 5 — unchanged.)

10. Color-mode select has no accessible label (confidence 90)
packages/dashboard/src/client/components/theme-gallery.tsx:53-59#pref-color-mode is wrapped in <span class="setting-label">, so the control has no accessible name, while the new font row directly below correctly uses <label class="setting-label" for="pref-font">. Pre-existing, but the PR's own addition demonstrates the fix. (Round-1 finding 4 — unchanged.)

11. Duplicated probe-element lists and layout-force loops in the browser suite (confidence 82)
packages/dashboard/test/client/appearance.browser.test.ts:493-495, 500-504, 569-571, 596-610, 615-619["body-text", "mono", "control", "memory"] is written out three-plus times and the void offsetHeight layout-force loop three times. A CORE_PROBE_IDS constant + forceProbeLayout(page, ids) helper (following the file's existing applyRoot style) would dedupe.

Strengths

  • The round-1 merge blocker (OpenDyslexic addendum) is fully implemented and independently verified end-to-end. Four unmodified WOFF2 faces byte-identical to the pinned upstream commit 449a41b (re-downloaded and SHA-256-compared; hashes also match OPENDYSLEXIC-PROVENANCE.md); verbatim OPENDYSLEXIC-OFL.txt (byte-identical to upstream OFL.txt at the pinned revision); four @font-face rules with contiguous non-overlapping 100 499 / 500 900 ranges so dashboard weights 500–800 use the real bold face (no faux synthesis); final [data-font="opendyslexic"] stack exactly per the addendum; package.json files + reproducible npm pack --dry-run shipping all four fingerprinted faces plus both legal files; all five doc surfaces consistently distinguish Google-hosted IBM from bundled JetBrains and bundled OpenDyslexic.
  • Round-1 deferred finding 7 was resolved for free: the font-only → no-color-scheme parity is now pinned at both layers (unit setter test + font-only first-paint bootstrap test).
  • The CSS cascade is statically airtight: all --mono-font declarations are (0,1,0) single-attribute selectors; the explicit [data-font] mappings sit at the very end of themes.css (loaded last) with no !important anywhere, so an explicit choice wins on <html> and scoped preview cards alike; every card always carries a non-theme data-font, preserving the no-eager-JetBrains invariant structurally.
  • Minimal error surface: guarded reads/writes, validator-normalized setters, allowlist-validated bootstrap (fully try/catch-wrapped), in-memory-authoritative fallback — every path pinned by an executed passing test; the font-display: swap failure path has parity with the pre-existing JetBrains behavior.
  • The tui hermeticity fix (1420281) is correct, minimal, disclosed, and verified under simulated SSH conditions: withEnv(…, undefined) deletes and restores in finally; the production guard checks exactly the three vars the test clears; 50/50 passing with all three SSH_* vars set. Test-only change; consistent with the repo's "no pre-existing failures" rule.
  • All suites green at head (re-executed by parent and specialists): 38 unit + 245 component + 100 real-Chromium dashboard tests; 50/50 tui keys; OpenDyslexic browser tests genuinely executed (network + document.fonts load-state oracles, not CSS string matching).

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Review Assessment

#487 (comment)

Independent verification at HEAD 142028160d029da4394102a427d96d012c85bf0f (clean tree, working tree == reviewed commit): the round-1 merge blocker (OpenDyslexic addendum) is fully resolved — all 9 addendum acceptance criteria re-verified against actual code, assets, build output, packaging, tests, and docs, including byte-level asset provenance (all four WOFF2 SHA-256s match OPENDYSLEXIC-PROVENANCE.md and fresh downloads of the pinned upstream commit 449a41b; OPENDYSLEXIC-OFL.txt byte-identical to upstream OFL.txt), reproducible npm pack --dry-run (four fingerprinted faces + both legal files), real-Chromium request isolation and four-face load states, and consistent three-way doc distinction across all five surfaces. Round-1 deferred finding 7 (font-only → no color-scheme parity) was also resolved by the addendum work (pinned at both unit and first-paint layers). The second commit (1420281, tui keys test hermeticity) is test-only, disclosed, and correct (pre-fix fails / post-fix passes under simulated SSH, independently reproduced; the guard at keys.ts:668 checks exactly the three vars the test clears). The independent assessor ran its own full-diff pass and found no new blocker-class issue.

Classifications

Finding Classification Reasoning
1. toContain on computed fontFamily false-pass shape useful follow-up Factual: verified — gruvbox's stack (themes.css:519) contains "IBM Plex Mono" as a fallback, so the row's family assertion would pass if the explicit override lost the cascade. Scope: no acceptance criterion mandates the assertion shape. Practical: no regression escapes at HEAD — every realistic losing-override shape is caught by the same row's network assertion (JetBrains-first computed stack fires a real jetbrains-mono.woff2 fetch, failing fontRequests === []), or directly by the solarized×jetbrains row's family assertion. Weak-oracle confidence issue for future rows, not a shipping risk. Follow-up: assert the computed stack's first family.
2. OpenDyslexic glyph-availability assertion near-tautological; minor ◆ attribution inaccuracy useful follow-up Factual: confirmed empirically — document.fonts.check is fallback-aware (returns true even when the named family lacks the glyph), so the stack-based check reduces to "host monospace covers the codepoints" for the glyphs OpenDyslexic lacks and cannot attribute which font renders which glyph. Scope: the addendum criterion requires glyphs available through the stack (met), and the fallback path is documented by design in OPENDYSLEXIC-PROVENANCE.md; the OpenDyslexic-specific half of the criterion is pinned by the non-tautological faceStates load assertions (all four faces loaded with exact 100 499/500 900 ranges — a broken or glyph-less asset would fail there). Practical: no user or security harm; independent pixel-diff probing of the shipped faces confirmed ●, ○ and all coding punctuation are genuine OpenDyslexic glyphs. Note: the same probe shows ◆ (U+25C6) is in the face, so the provenance and in-test comment overstate the fallback set (✕ and ↻ are the true fallback cases) — conservative direction, no behavioral or licensing impact. Follow-up: stronger per-glyph attribution (check against OpenDyslexic alone for the glyphs it claims) + correct the ◆ wording.
3. tui hermeticity fix lacks a meta-guard for future guard vars deferred Factual: verified — guard is WT_SESSION && !SSH_CONNECTION && !SSH_CLIENT && !SSH_TTY (keys.ts:668); the test clears exactly those three. Scope: tui tests are outside the PR's authoritative scope; the fix itself is consistent with the repo's no-pre-existing-failures rule. Practical: correct at HEAD (pre-fix 49/50 → post-fix 50/50 under simulated SSH, independently reproduced); the described harm is a future silent re-break if a fourth guard var is added — missing meta-test is not a blocker. Follow-up: clear all SSH_* vars in the test so it tracks guard evolution.
4. Request-capture boilerplate ×4 / near-clone preview test nitpick Factual: verified (six capture blocks; the OpenDyslexic preview test is a line-for-line clone of the JetBrains one). Scope: none. Practical: test-only style; blocks are correct and low-hazard. A captureRequests(page, isMatch) helper would fit the file's convention.
5. Theme-default matrix asserts computed family only for #mono deferred Factual: verified — unchanged since round 1 (matrix reads #mono only; body/control/memory asserted only in explicit-font tests). Scope: no criterion mandates per-element theme-default assertions. Practical: every realistic regression shape is caught by another passing assertion; the only escaping shape (element-specific font-family active solely when data-font is absent) is contrived. Same analysis as round 1; missing test alone is not a blocker.
6. Render-blocking Google Fonts stylesheet deferred Factual: verified — synchronous external fonts.googleapis.com link at index.html:19-25, byte-unchanged by this PR. Scope: the plan explicitly adopts the Google-hosted IBM design and all five doc surfaces describe it; the network contract is identical in every font state. Practical: an offline/CDN-blocked user experiences the same pre-existing delay and the page still renders via the Courier New, monospace fallback — no new failure path, no new attacker surface. Legitimate follow-up (self-host IBM or non-blocking load), not a blocker here.
7. Bootstrap indexOf allowlist checks evaluated twice nitpick Factual: verified (index.html:49-52). Scope: none. Practical: four tiny-array indexOf calls per page load; negligible. Micro-style preference (round-1 finding 8, unchanged).
8. Cross-tab storage guard negation clauses nitpick Factual: verified (appearance.ts:268-279); logic correct and unit-tested. Scope: none. Practical: readability preference only (round-1 finding 10, unchanged).
9. theme-gallery.tsx header doc omits data-font nitpick Factual: verified (header lines 3-14 vs inline comment 86-90). Scope: internal comment; public docs accurate. Practical: no user-visible effect (round-1 finding 5, unchanged).
10. Color-mode select has no accessible label nitpick Factual: verified — <span class="setting-label"> at theme-gallery.tsx:53-59; the PR's new font row uses the correct <label for> pattern. Scope: pre-existing, untouched by this PR; no criterion covers a11y labeling. Practical: neither introduced nor worsened by this PR (round-1 finding 4, unchanged). Optional one-liner for a future PR.
11. Duplicated probe-element lists / layout-force loops nitpick Factual: verified in the browser suite. Scope: none. Practical: test-only duplication, currently correct.

Prior-round verification

  • Round-1 finding 1 (merge blocker — OpenDyslexic addendum unimplemented): RESOLVED. All 9 addendum acceptance criteria met and independently verified (catalog order; bootstrap allowlist + exact-array contract test; four unmodified pinned upstream WOFF2 faces — byte-identical to fresh upstream downloads; four @font-face ranges mapping 500–800 to the real bold face; final [data-font="opendyslexic"] stack per the addendum; verbatim OFL + provenance with matching checksums; package.json files + reproducible npm pack --dry-run; five consistent doc surfaces; extended unit/component/browser tests green — 38 + 245 + 100 at HEAD).
  • Round-1 finding 7 (deferred — font-only color-scheme parity unpinned): RESOLVED by the addendum work — pinned at both the unit and first-paint-bootstrap layers.
  • Round-1 findings 2, 6 (deferred): unchanged — still open as deferred follow-ups (classifications 5 and 6 above).
  • Round-1 nitpicks 4, 5, 8, 9, 10: unchanged (classifications 10, 9, 7, 4, 8 above).

Action Plan

No merge blockers. Every candidate fails at least one gate (none is both required-by-scope and materially harmful to ship), and the sole prior merge blocker is fully resolved and independently verified at HEAD. The PR is ready to merge.

Deferred follow-ups (outside this PR's scope, optional): 1 (assert computed stack head), 2 (stronger glyph attribution + ◆ wording fix), 3 (SSH_* meta-guard), 5 (theme-default matrix element breadth), 6 (self-host or non-blocking load of IBM Plex Mono).


Assessment by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Implementation Plan Addendum — Three Additional Bundled Font Families

Rescope requested by the maintainer in this thread (2026-08-28): add Fira Code, Iosevka, and Atkinson Hyperlegible to the font picker alongside the existing four options (Theme default, IBM Plex Mono, JetBrains Mono, OpenDyslexic). The font pipeline is catalog-driven — persistence, validation, bootstrap, cross-tab sync, preview scoping, and request isolation all generalize automatically — so each family is mechanical: catalog entry, bootstrap id, @font-face rules, one final [data-font] mapping, assets + license + provenance, docs, and test rows.

Resolved product and technical choices

  • Picker order: Theme default → IBM Plex Mono → JetBrains Mono → Fira Code → Iosevka → OpenDyslexic → Atkinson Hyperlegible (monospace families first; the explicit accessibility options last, consistent with OpenDyslexic's position). Storage ids: fira-code, iosevka, atkinson-hyperlegible.
  • All three bundled, self-hosted, lazy-loaded (no CDN, no preload) — extending the existing hosting model. Explicit selection overrides any theme via the final [data-font] mappings; Theme default semantics are unchanged.
  • Weights: exact single-weight @font-face declarations at 400/500/600 (+400 italic where the family ships italics) — the exact weights the dashboard CSS uses, same shape as the Google-hosted IBM faces. No weight ranges, no faux synthesis of declared weights.
  • CSS family names: "Fira Code", "Iosevka", "Atkinson Hyperlegible Next" (the Atkinson v2.001 name table uses per-weight family names; the @font-face rules deliberately declare a single family). Fallback stacks: each family + "IBM Plex Mono", "Courier New", monospace.
  • Fira Code (3 faces): upstream ships TTFs only → re-encoded to WOFF2 (fonttools 4.63.0), not subsetted (~103 KB each). Ligatures ship in calt (default-on in browsers). No italic face exists upstream — italic text renders synthesized; documented as an upstream limitation in the provenance.
  • Iosevka (4 faces): upstream release TTFs are ~7.7 MB each → subset to the dashboard's unicode needs (Latin/Extended, Greek, Cyrillic, coding punctuation, arrows, math, geometric shapes, dingbats — including every dashboard status glyph ● ◆ ○ ✕ ↻) and re-encoded (~91 KB each); layout features calt/ccmp/liga/dlig/zero retained, ~100 stylistic cvNN/vertical-writing features pruned. No OFL Reserved Font Name, so subsetting is unrestricted (same pattern as the JetBrains files).
  • Atkinson Hyperlegible (4 faces): the 2024 second-generation official version ("Next", v2.001) from googlefonts/atkinson-hyperlegible-next; upstream ships ready-to-serve WOFF2, used unmodified (~25 KB each). Picker label "Atkinson Hyperlegible". Latin-centric by design; coverage gaps (Greek/Cyrillic, arrows, status shapes) fall back within the stack, documented in provenance.
  • Tarball growth ≈ 787 KB across the 11 new faces; zero runtime transfer unless a family is selected.
  • Provenance records pin exact upstream revisions (Fira Code release 6.2; Iosevka tag v34.8.1; Atkinson commit 7925f50f), source URLs, input and output SHA-256, processing steps, face/weight mapping, and per-family glyph coverage.

Acceptance criteria

  • Settings offers the seven options in the order above; each applies immediately, persists per browser, restores before first paint, and is independent of theme/color mode.
  • Theme default remains the no-key/no-attribute case and retains the current IBM/Gruvbox mapping.
  • Opening Settings in Theme default (gruvbox included — the strongest case) requests no face of any new family.
  • Selecting a new family: the computed stack on body/code/controls/memory editor starts with that family on every theme (including gruvbox, where the selection must also cause zero jetbrains-mono.woff2 requests); every bundled face is fetched from the local origin only and reaches loaded state (no faux-bold of a declared weight; real italics where shipped); no CDN request.
  • Fira Code renders its default-on calt ligatures; Iosevka's subset covers every dashboard status glyph.
  • Invalid/stale stored values still fall back safely to Theme default.
  • The npm package contains all 11 built WOFF2 assets plus the three verbatim OFL files and three provenance records.
  • All five documentation surfaces describe the shipped 7-option behavior consistently and accurately distinguish Google-hosted IBM Plex Mono from the five bundled families.

Verification performed

Biome clean · npm run build emits all 11 fingerprinted faces · focused suites green (39 unit + 245 component + 105 real-browser, incl. new per-family cascade/load/isolation/glyph tests) · full monorepo suite green · npm run verify-workspace-links clean · npm pack --dry-run lists all 11 faces + 6 legal/provenance files · fontTools verification of the shipped binaries (weight classes, italic flags, GSUB features, cmap coverage vs the documented claims).


Plan addendum created for the maintainer-requested font rescoping

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Progress Update — maintainer-requested rescope: three additional bundled fonts

Per the maintainer's rescoping decision in the PR thread, Fira Code, Iosevka, and Atkinson Hyperlegible are now implemented as the 5th/6th/7th font-picker options (full plan addendum posted above).

What landed

  • Picker: seven options in order — Theme default, IBM Plex Mono, JetBrains Mono, Fira Code, Iosevka, OpenDyslexic, Atkinson Hyperlegible (monospace first, accessibility options last). All existing lifecycle behavior (persistence, validation, first-paint bootstrap, cross-tab sync, preview scoping, no-eager-fetch) extends unchanged.
  • Assets (11 new WOFF2 faces, ~787 KB total, lazy-loaded only when selected):
    • Fira Code 6.2 — 3 faces (400/500/600), TTF→WOFF2 re-encoded, unsubsetted; calt ligatures default-on. No italic face exists upstream — documented as an upstream limitation (synthesized oblique for explicit Fira Code users).
    • Iosevka v34.8.1 — 4 faces (400/500/600 + italic), subset to the dashboard's unicode needs (all dashboard status glyphs included) + re-encoded (~91 KB each vs ~1.1 MB unsubsetted); ligatures preserved (calt/dlig/zero), ~100 stylistic features pruned.
    • Atkinson Hyperlegible Next 2.001 (the 2024 second-generation official version) — 4 upstream WOFF2 faces used unmodified (~25 KB each).
  • Provenance & licensing: each family pinned to an exact upstream revision (release/tag/commit) with source URLs, input + output SHA-256, processing steps, face/weight mapping, and per-family glyph-coverage notes; verbatim OFL files (no Reserved Font Name in any of the three). package.json files allowlist extended; npm pack --dry-run verified (all 11 fingerprinted faces + 6 legal/provenance files).
  • Docs: all five public surfaces updated for the 7-option behavior, distinguishing Google-hosted IBM Plex Mono from the five bundled families.
  • Tests: catalog/bootstrap contracts (7 options, exact 6-element allowlist), settings option order, and new real-browser coverage per family: explicit selection must make the computed stack start with the chosen family on body/code/controls/memory editor (on gruvbox — where a losing cascade would also fetch JetBrains, now asserted zero), all faces reach loaded with single-weight faces (no faux synthesis), zero CDN requests, theme-default mode requests no new face, and family-alone glyph checks against exactly the glyphs each provenance record claims.

Verification: Biome clean · build emits all 11 faces into dist · focused suites green (39 unit + 245 component + 105 real-browser) · full monorepo suite green (5,902 passed, pre-commit) · workspace links valid · fontTools verification of shipped binaries (weight classes, italic flags, GSUB features, cmap coverage vs the documented claims).

Commit: b74ff61


Progress tracked by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Unverified Review Candidates — Pending Assessment

Review round: 3
Reviewed commit: b74ff61
Delta base: 1420281

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Round-3 scope note: review was restricted to commit b74ff61 and its interactions. The round-1 missing-OpenDyslexic blocker remains resolved; round 2 had no merge blockers. Unchanged observations were rejected unless this rescope made them newly reachable.

Critical

None.

Important

1. Explicit bundled selections still contact the Google Fonts CDN in production (confidence 95)
The addendum requires an explicit new bundled family to make no CDN request, but packages/dashboard/src/client/index.html:20-25 unconditionally loads the Google-hosted IBM Plex Mono stylesheet before the persisted data-font choice can suppress it. The browser assertion at packages/dashboard/test/client/appearance.browser.test.ts:743-814 uses a synthetic local-only font-probe.html, so it cannot observe the production request. This is newly in scope because the round-3 acceptance criterion explicitly promises no CDN request for each newly bundled selection.

2. Iosevka provenance claims a liga feature absent from every shipped face (confidence 99)
packages/dashboard/src/client/assets/fonts/IOSEVKA-PROVENANCE.md:71-73 says calt, ccmp, liga, dlig, and zero were retained. Direct fontTools inspection found calt, ccmp, dlig, and zero, but no liga, in all four shipped WOFF2 files; the pinned upstream inputs also lack it. The addendum names liga as retained, so either the requirement/provenance must be corrected or the binaries must provide it.

3. Two public documentation surfaces omit the required hosting distinction (confidence 98)
README.md:150 and packages/coding-agent/README.md:84 list the seven options but do not distinguish Google-hosted IBM Plex Mono from bundled JetBrains Mono, Fira Code, Iosevka, OpenDyslexic, and Atkinson Hyperlegible. The addendum explicitly requires all five documentation surfaces to make that distinction; the dashboard guide, dashboard package README, and font provenance do.

4. New glyph-coverage assertion cannot prove glyph presence in a bundled face (confidence 100)
packages/dashboard/test/client/appearance.browser.test.ts:842-879 uses document.fonts.check() with a family alone, but Chromium can return true when the matching face is loaded even if supplied characters are absent and will fall back. Removing a required status glyph from the Iosevka subset could therefore leave the test green. Parse each shipped WOFF2 cmap and assert the claimed code points, especially ● ◆ ○ ✕ ↻.

5. Face-state checks trust CSS descriptors rather than binary weight/style metadata (confidence 98)
packages/dashboard/test/client/appearance.browser.test.ts:794-807 reads FontFace.style and FontFace.weight, which reflect the @font-face declarations rather than internal font metadata or outlines. Swapping a medium file for regular, or an upright file for italic, could still pass while violating the real-weight/real-italic requirement. Verify checksums and/or OS/2 weight plus italic metadata for all eleven binaries.

6. Fira Code's required default-on ligatures are not tested (confidence 99)
The tests prove the faces load and contain punctuation, but do not verify that calt survived conversion or remains enabled by default. Dropping the GSUB feature or disabling contextual alternates would leave the suite green while the advertised =>/!= ligatures stop rendering. Add a binary feature check and, ideally, a browser rasterization comparison with calt enabled versus disabled.

7. Lazy-loading coverage permits production preloads and cross-family overfetch (confidence 97)
The no-request test runs synthetic font-probe.html, so a production <link rel="preload"> regression would escape. Explicit-family rows require the expected filenames to be present but do not require the request set to equal that family's faces, so selecting Fira Code could also fetch all Iosevka and Atkinson faces while passing. This could transfer most of the added ~787 KiB unnecessarily. Exercise production HTML for no-preload behavior and assert exact per-family request sets.

8. Published-package contents have no automated contract (confidence 99)
packages/dashboard/package.json:21-33 was updated and npm pack --dry-run currently succeeds, but no test inspects the built package. Removing an OFL/provenance allowlist entry or failing to emit a fingerprinted WOFF2 could leave all focused tests green while the published package violates the explicit asset/legal acceptance criterion. Add a packaging contract around npm pack --dry-run --json or equivalent manifest inspection.

Suggestions

9. “Across themes” cascade behavior is sampled only on Gruvbox (confidence 90)
packages/dashboard/test/client/appearance.browser.test.ts:743-815 tests each new family only on Gruvbox. It is the strongest current precedence case, but a future theme-specific/later rule affecting another theme could restore that theme's family while these rows stay green. Consider a matrix over every theme for the four representative consumers.

Strengths

  • The seven-option picker order, IDs, bootstrap allowlist, persistence, CSS mappings, and theme/color independence are consistent.
  • All 11 new faces are local and lazy at the reviewed head; build output and package dry-run contain the expected assets and legal/provenance files.
  • Atkinson files match the pinned upstream WOFF2 files byte-for-byte; OFLs match upstream; documented checksums match the shipped assets.
  • Fira Code retains functional calt; Iosevka includes every required dashboard status glyph and the other four documented layout features.
  • Focused unit, component, and real-browser suites pass (389 tests), the tree is clean, workspace links are valid, and OpenDyslexic behavior remains intact.
  • Runtime/storage failure handling remains guarded and degrades safely through explicit fallback stacks.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker


Reviewed by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Review Assessment

#487 (comment)

Assessed at exact HEAD b74ff613ff76860f97d140744d59263638ad7ca7 against round-3 base 142028160d029da4394102a427d96d012c85bf0f. The tree is clean. The prior OpenDyslexic blocker remains resolved, round 2 had no blockers, the focused dashboard suites pass, and build/package inspection confirms the current assets ship correctly.

Round-3 counter-pressure produced disagreement on findings 1 and 2. Under the round-3 rule, a candidate blocks only when both the independent assessor and developer's advocate find material practical impact. They agreed only on finding 3.

Classifications

Finding Classification Reasoning
1. Explicit bundled selections still contact Google Fonts deferred Factual: production index.html:20-25 unconditionally requests the IBM stylesheet; the synthetic probe cannot observe it. Scope: ambiguous rather than blocker-clear: the addendum says a selected new family causes “no CDN request,” but also explicitly preserves the existing hosting model and requires docs to distinguish Google-hosted IBM from bundled families; this exact unchanged link was deferred in rounds 1–2. Practical adjudication: actor = every dashboard user; trigger = any production page load, independent of selected font; outcome = the same pre-existing Google request/privacy/offline dependency; safeguard = bundled faces and their bytes remain local and lazy, and fallback rendering still works. The delta neither creates nor increases the request. Removing/gating it would materially improve privacy/offline behavior, but the independent assessor did not find round-3 material impact attributable to this delta, so it cannot block under the required dual-assessor rule.
2. Iosevka provenance claims absent liga useful follow-up Factual: confirmed — all four shipped faces and pinned upstream inputs lack liga; IOSEVKA-PROVENANCE.md says it was retained. Scope: the addendum names the feature and requires accurate provenance. Practical adjudication: actor = maintainer/auditor; trigger = relying on the provenance feature list; outcome = one false technical statement, but no rendering loss because functional default ligatures use the present calt feature. Safeguards = direct binary verification confirms calt, ccmp, dlig, and zero, and no runtime code consumes the prose. Correcting one line is worthwhile, but the developer's advocate found no material user impact, so round-3 blocker consensus is absent.
3. Two public README surfaces omit hosting distinction merge blocker Factual: README.md:150 and packages/coding-agent/README.md:84 list the options but do not distinguish Google-hosted IBM Plex Mono from the five bundled families; the deeper dashboard docs do. Scope: directly violates the explicit categorical criterion that all five documentation surfaces accurately make this distinction, plus the repository rule that the root README must remain accurate. Practical: actor = user/deployer reading either primary product README; trigger = evaluating font network/offline/privacy behavior from the documented font list; outcome = they cannot tell which choice is Google-hosted and which five ship locally. Deeper docs are a partial safeguard but do not satisfy the promised five-surface consistency. Both required round-3 assessors found material impact; fixing two sentences gives users the promised information and completes the authorized docs work.
4. Glyph assertion cannot prove cmap coverage useful follow-up Factual: document.fonts.check() is not a reliable per-codepoint cmap oracle; direct fontTools inspection nevertheless confirms all required Iosevka status glyphs are present at HEAD. Scope: actual glyph coverage is required and currently satisfied; a specific test method is not. Practical: only a future re-subset regression could escape and fallback limits visible harm. A direct cmap assertion would strengthen the guard but changes no current behavior.
5. Face-state checks trust CSS descriptors useful follow-up Factual: FontFace.weight/style reflect CSS descriptors, not internal metadata; direct inspection confirms all current binaries have the intended weights/italic flags. Scope: real declared weights/italics are required and currently present. Practical: checksum/OS2 assertions would catch a future asset swap, but there is no current faux-face defect.
6. Fira Code calt is untested useful follow-up Factual: no test directly checks GSUB/browser ligature substitution; direct inspection confirms calt exists in all shipped Fira faces and current CSS does not disable it. Scope: the behavior is required and currently satisfied. Practical: an additional assertion prevents a future conversion/CSS regression but fixes no present user-visible failure.
7. Lazy-loading tests permit preloads/cross-family overfetch discarded observation Factual: the assertions would not catch those hypothetical future shapes, but source and built output have no bundled-font preload, no eager FontFace() path, and no current cross-family overfetch. Scope/Practical: current lazy-loading behavior is correct; this is test hardening for nonexistent paths. The real production-CDN concern is already classified as finding 1.
8. Package contents lack an automated contract useful follow-up Factual: no automated pack-manifest assertion exists. Scope: actual package contents are required and verified correct now: all 11 new fingerprinted WOFF2 assets plus the six new OFL/provenance files are present. Practical: automation would prevent future release drift, but missing automation alone is not a blocker.
9. Cascade is sampled only on Gruvbox discarded observation Factual: only Gruvbox is used in the new matrix. Scope/Practical: the explicit [data-font] mappings are one generic end-of-file block with equal specificity and later source order than every theme mapping; no theme-specific combined rule exists. Gruvbox is the strongest case because its default differs. A full matrix would repeat invariant cascade math without addressing a current gap.

Prior-round verification

  • Round-1 finding 1 (OpenDyslexic missing): resolved and not regressed. The catalog, bootstrap, assets, faces, package/legal records, docs, and browser behavior remain present.
  • Round 2: no merge blockers existed at the reviewed base.
  • Unchanged round-1/2 observations were not revived as blockers unless the delta made them newly reachable; the Google stylesheet remains deferred after explicit round-3 adjudication.

Action Plan

  1. Fix finding 3: update README.md and packages/coding-agent/README.md so both explicitly distinguish Google-hosted IBM Plex Mono from bundled/self-hosted JetBrains Mono, Fira Code, Iosevka, OpenDyslexic, and Atkinson Hyperlegible, matching the other required documentation surfaces.

Everything else remains outside the merge-blocker action plan.


Assessment by mach6

@maxscheurer

Copy link
Copy Markdown
Contributor Author

Progress Update — finding 3 resolved

Updated both remaining public documentation surfaces so the seven-option font picker now clearly distinguishes Google-hosted IBM Plex Mono from the five bundled self-hosted families: JetBrains Mono, Fira Code, Iosevka, OpenDyslexic, and Atkinson Hyperlegible.

Files:

  • README.md
  • packages/coding-agent/README.md

Verification: full build passed · all workspace links valid · diff check clean · pre-commit suite passed (5,903 tests, 0 failures)

Commit: 0cfee34


Progress tracked by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Unverified Review Candidates — Pending Assessment

Review round: 4
Reviewed commit: 0cfee34
Delta base: b74ff61

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Prior-blocker verification: round-3 finding 3 is fixed. README.md:150 and packages/coding-agent/README.md:84 now explicitly distinguish Google-hosted IBM Plex Mono from the five bundled self-hosted families. The round-1 OpenDyslexic blocker remains resolved; round 2 had no blockers.

Critical

None.

Important

1. Explicit bundled selections still contact Google Fonts in the production page (confidence 99)
packages/dashboard/src/client/index.html:20-25 unconditionally loads the Google Fonts stylesheet, while the no-remote-request browser assertions in packages/dashboard/test/client/appearance.browser.test.ts:633-679,743-812 use a synthetic font-probe.html that omits that production dependency. An actual dashboard load therefore still contacts Google when any bundled family is selected, despite the latest addendum's explicit “no CDN request” criterion. A production-HTML browser test seeded with each bundled preference would expose the current behavior. This is the round-3 CDN candidate re-reported after full-PR review; unchanged lines remain in scope.

2. The Fira Code SemiBold input SHA-256 is malformed (confidence 99)
packages/dashboard/src/client/assets/fonts/FIRACODE-PROVENANCE.md:29 records a 62-character digest for FiraCode-SemiBold.ttf; a SHA-256 digest must contain 64 hexadecimal characters. The output WOFF2 digest is well-formed, but an auditor cannot reproduce or verify the documented input-to-output chain of custody for this face. Replace it with the correct digest from the pinned Fira Code 6.2 release input.

3. Iosevka provenance claims a liga feature absent from the inputs and shipped faces (confidence 99)
packages/dashboard/src/client/assets/fonts/IOSEVKA-PROVENANCE.md:71-73 says liga was retained. Prior direct fontTools inspection found no liga feature in any of the four shipped faces or pinned upstream inputs; calt, ccmp, dlig, and zero are present. Runtime ligatures work through calt, but the provenance record contains a false technical statement. Remove liga from the retained-feature list unless the assets are changed to provide it.

Suggestions

4. Storage reads duplicate the same guarded validation pipeline three times (confidence 83)
packages/dashboard/src/client/state/appearance.ts:111-141 has structurally identical readThemeStorage, readModeStorage, and newly added readFontStorage functions, while writes are already centralized in writeSetting. A generic typed readStorage(key, validator, defaultValue) would reduce drift risk without changing behavior.

5. The color-scheme ternary can express only the exceptional mode (confidence 85)
packages/dashboard/src/client/state/appearance.ts:210 maps light and dark to the same strings through a chained ternary. m === "system" ? "light dark" : m is equivalent and clearer.

6. The storage-event guard should use an owned-key allowlist (confidence 80)
packages/dashboard/src/client/state/appearance.ts:283-296 now has one negated conjunct per appearance key. A shared set or array of owned storage keys would state the intent directly and scale without extending the condition chain.

7. Bootstrap theme/mode validity checks are evaluated twice (confidence 82)
packages/dashboard/src/client/index.html:49-54 repeats the same THEMES.indexOf(t) and MODES.indexOf(m) checks for attribute assignment and the active calculation. Hoisting validTheme and validMode booleans would remove duplication and clarify the color-scheme condition.

Strengths

  • The round-3 documentation blocker is resolved surgically at the exact two missing primary surfaces; all five required documentation surfaces now describe the seven options and hosting distinction consistently.
  • The seven-option catalog, bootstrap allowlist, persistence lifecycle, CSS override layer, bundled assets, package allowlist, and browser interaction coverage remain internally consistent.
  • Storage and DOM boundaries are guarded; invalid persisted values normalize to Theme default; bootstrap/catalog drift has exact contract tests; font-only selection consistently does not activate color-scheme.
  • All referenced WOFF2 paths exist and have valid WOFF2 signatures, with legible fallback stacks and font-display: swap for asset-loading failures.
  • The bundled-family browser matrix is catalog-driven, checks stack precedence on representative consumers, and verifies local face loading without copy-pasted family-specific suites.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Review Assessment

#487 (comment)

Assessed at exact clean HEAD 0cfee34c202b587bd08f73e7451ff8dc01914aa5, with the full PR as the review target and b74ff613ff76860f97d140744d59263638ad7ca7..HEAD used only as supplemental fix-verification context. The round-3 documentation blocker is resolved: both primary READMEs now distinguish Google-hosted IBM Plex Mono from the five bundled self-hosted families. The round-1 OpenDyslexic blocker remains resolved, and round 2 had no blockers.

Under the round-4 rule, a finding can block only when both the independent assessor and developer's advocate find material practical impact. They agreed on no candidate. Findings 2 and 3 are factual, in-scope provenance inaccuracies, but the developer's advocate found no material user, runtime, security, or release-integrity impact sufficient to block; the parent agrees after concrete trigger-and-outcome adjudication below.

Classifications

Finding Classification Reasoning
1. Explicit bundled selections still contact Google Fonts deferred Factual: production index.html:20-25 unconditionally loads the existing IBM Plex Mono stylesheet, and the synthetic probe cannot observe that request. Scope: the original plan explicitly preserved Google-hosted IBM, all five current docs disclose it, and the bundled-family locality assertions prove the selected bundled faces themselves are same-origin; this does not establish an addendum violation. Practical: actor = any dashboard user; trigger = any production page load, regardless of selected font; outcome = the same pre-existing Google connection/privacy/offline dependency, with Courier New, monospace as the rendering safeguard. The PR does not create or increase the request. Self-hosting or gating IBM would improve privacy/offline behavior, but is a separate follow-up, not a blocker.
2. Fira Code SemiBold input SHA-256 is malformed useful follow-up Factual: confirmed — FIRACODE-PROVENANCE.md:29 contains a 62-character value where SHA-256 requires 64 hexadecimal characters. Scope: accurate input/output checksums are part of the approved provenance deliverable. Practical: actor = maintainer or auditor; trigger = downloading the pinned Fira Code 6.2 input and comparing its SemiBold digest; outcome = that one input-to-output chain cannot be verified from the recorded value. Safeguards = the release/source pin remains available and the shipped WOFF2 output digest is well-formed and independently verifiable. Correcting the line materially improves auditability, but no supported user behavior, shipped font bytes, security control, or rendering path fails. The developer's advocate found no blocker-level practical impact, so round-4 consensus is absent.
3. Iosevka provenance claims an absent liga feature useful follow-up Factual: confirmed — all four shipped faces expose calt, ccmp, dlig, and zero, not liga, while IOSEVKA-PROVENANCE.md:71-73 says liga was retained. Scope: accurate feature provenance is part of the approved deliverable. Practical: actor = maintainer or auditor; trigger = comparing the prose with the binaries; outcome = one false feature claim. Safeguards = functional default ligatures use the present calt; no runtime code consumes the prose and no rendering capability is missing. Removing one token improves accuracy, but produces no material user or system outcome; the required round-4 blocker agreement is absent.
4. Storage reads duplicate the guarded pipeline nitpick Factual: the three readers are structurally similar. Scope: no acceptance requirement mandates a generic helper. Practical: all paths are correct, guarded, and tested; centralization trades explicit functions for generic indirection without changing behavior.
5. color-scheme chained ternary nitpick Factual: the proposed expression is equivalent. Scope: none. Practical: style-only substitution with no observable outcome; the current form also mirrors bootstrap behavior.
6. Storage-event guard allowlist nitpick Factual: the guard has one negated clause per owned key. Scope: none. Practical: current logic is correct and tested for owned, unrelated, and null keys; an allowlist is a readability preference only.
7. Bootstrap validity checks evaluated twice nitpick Factual: theme and mode indexOf checks each run twice. Scope: none. Practical: two tiny-array scans once per page load are negligible; hoisting booleans changes style, not supported behavior.

Prior-round verification

  • Round-3 finding 3: resolved. README.md:150 and packages/coding-agent/README.md:84 now provide the required Google-hosted-versus-bundled distinction.
  • Round-1 OpenDyslexic blocker: remains resolved. Catalog, assets, CSS mappings, legal/provenance records, package entries, and tests remain present.
  • Round 2: no merge blockers existed.

Action Plan

No merge blockers. Findings 2 and 3 are worthwhile provenance corrections but fail the round-4 practical-impact consensus required for the merge-blocker action plan. Finding 1 remains a separate deferred privacy/offline improvement. Findings 4–7 are nitpicks.


Assessment by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator

Progress Update — font provenance corrections

Resolved review findings 2 and 3:

  • Corrected the malformed Fira Code SemiBold input SHA-256 to the verified 64-character digest from the pinned Fira Code 6.2 release archive.
  • Removed the nonexistent liga feature from the Iosevka retained-feature list; all four shipped faces provide calt, ccmp, dlig, and zero.

Verification: pinned archive checksum and input digest verified · shipped Iosevka GSUB features inspected · diff check clean · full build passed · full test suite passed · workspace links valid · pre-commit suite passed (5,902 tests, 0 failures)

Commit: 0d398d7


Progress tracked by mach6

@m-aebrer
m-aebrer merged commit 7493863 into aebrer:master Aug 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add font selection to the dashboard

2 participants