Skip to content

fix(dashboard): inline critical-CSS bootstrap for user themes to mitigate flash - #36024

Closed
nnnet wants to merge 1 commit into
NousResearch:mainfrom
nnnet:fix/theme-fouc-css-bootstrap
Closed

fix(dashboard): inline critical-CSS bootstrap for user themes to mitigate flash#36024
nnnet wants to merge 1 commit into
NousResearch:mainfrom
nnnet:fix/theme-fouc-css-bootstrap

Conversation

@nnnet

@nnnet nnnet commented May 31, 2026

Copy link
Copy Markdown
Contributor

Problem

User themes (`~/.hermes/dashboard-themes/*.yaml`) reach the SPA only after the `/api/dashboard/themes` round-trip completes at React mount. The bundle paints the first frame with the default Hermes Teal canvas because:

  • the bundled stylesheet (`<link rel="stylesheet">`) declares `:root { --background-base: #041c1c; ... }` for the built-in default;
  • the bundled `web/src/themes/presets.ts` exposes the same default-theme palette as the `ThemeProvider` initial state;
  • `ThemeProvider.applyTheme()` only fires once the API response arrives and `setUserThemeDefs` populates the lookup.

The user sees a green canvas behind the loading SPA on every reload whenever the active theme is a user theme. This makes the user-themes extension point — the documented way to ship a custom palette without rebuilding the bundle — feel second-class compared to built-ins.

Built-in themes do not have this problem. Their full definitions are bundled inside `presets.ts`, so the SPA owns the palette before first paint.

Solution (backend only)

`_serve_index()` injects a small critical-CSS `<style>` block inside `` whenever the active theme is a user theme. The block carries the six CSS variables that determine the canvas/text colour and base typography:

  • `--background-base`, `--color-background`
  • `--midground-base`, `--color-midground`
  • `--font-sans`, `--font-base-size`

…and an `html, body` rule that paints the canvas using those variables.

Because the inline `<style>` follows the bundle's `` in DOM order and matches the same `:root` specificity (0,0,1,0), the later declaration wins the CSS cascade. The static canvas behind the SPA is already in the user theme's palette before any JavaScript runs.

When `ThemeProvider` later mounts and `applyTheme()` writes the same variables as inline styles on `documentElement`, the values are identical to what the bootstrap block set — there is no second-paint discrepancy on the critical variables.

`_render_active_theme_bootstrap_css()` looks up the active theme through the existing `_discover_user_themes()` helper. No-op for built-in active themes (empty string returned, no `<style>` injected). No new endpoints, no config flags, no frontend changes.

Benefits

  • Eliminates the static green-canvas-behind-the-SPA on reload. The first paint is already in the user theme's palette.
  • No frontend rebuild required. Operators already running a user theme get the fix as soon as the backend image updates.
  • No regression for built-in themes. Active built-in returns an empty string from the helper; HTML output is byte-identical.
  • No new public surface. Reuses `_discover_user_themes()` / `load_config()` / `cfg_get()`; no API endpoints, no config keys, no breaking changes.
  • Trivial review surface. ~60 LOC backend change; the helper plus one `html.replace()` call.
  • Trust model unchanged. YAML themes are already trusted: `_normalise_theme_definition` accepts user-supplied `customCSS` up to 32 KiB. The bootstrap helper only emits well-known hex/font strings sourced from the same normalised definition, and `</` is escaped defensively inside the block.

Test plan

  • Manual: `~/.hermes/dashboard-themes/mission-control.yaml` (dark canvas) + `config.yaml dashboard.theme: mission-control`; reload — the canvas behind the SPA loads in the MC palette, no green flash.
  • Manual: switch to a built-in theme via the picker (`config.yaml dashboard.theme: midnight`), reload — `<style id="hermes-theme-bootstrap">` is not injected (View Source confirmed); bundle behaviour unchanged.
  • Manual: no user themes configured + built-in active — empty string returned by the helper, HTML output unchanged.
  • HTML response contains `<style id="hermes-theme-bootstrap">` immediately before the existing `bootstrap_script` block, in that order.
  • CI: a snapshot test on `_render_active_theme_bootstrap_css()` for built-in / user / missing-active / no-user-themes cases would catch regressions — happy to add in a follow-up if maintainers prefer.

Notes

The patch is intentionally conservative: it covers only the canvas / text / font variables. A residual short flash is still possible if the bundle's `ThemeProvider` re-applies the default theme as inline styles on `documentElement` between mount and the API response — that race is best fixed on the frontend (e.g. having `ThemeProvider` read a backend-injected JS global with the active theme's full definition). Filing that as a separate PR keeps each change focused and reviewable in isolation; this PR removes the static green-canvas case that every user-theme operator hits on every reload today.

…gate flash

User themes (`~/.hermes/dashboard-themes/*.yaml`) reach the SPA only
after `/api/dashboard/themes` resolves at React mount.  The bundle paints
the first frame with the default Hermes Teal canvas — the
`<link rel="stylesheet">` carries `:root{--background-base:#041c1c}`,
the bundled `presets.ts` defines the same surfaces in JS — and then
`ThemeProvider.applyTheme(<user theme>)` flips the inline CSS variables
on `documentElement` once the API response lands.  Visible to the user
as a green canvas behind the loading SPA on every reload when the active
theme is non-default.

Built-in themes do not suffer the same effect because their full
definitions ship inside the bundle, so the SPA already has the palette
before first paint.

This patch closes the gap on the backend side: `_serve_index()` injects
a `<style id="hermes-theme-bootstrap">` block inside `<head>` with the
six critical CSS variables (`--background-base`, `--color-background`,
`--midground-base`, `--color-midground`, `--font-sans`,
`--font-base-size`) plus an `html, body` rule painting the body in the
target palette.  Because the inline `<style>` follows the bundle's
`<link>` in DOM order and matches the same `:root` specificity, the
later declaration wins the cascade — the static canvas behind the SPA is
already the right colour before any JavaScript runs.

`_render_active_theme_bootstrap_css()` looks up the active theme through
the existing `_discover_user_themes()` helper.  No-op for built-in
active themes (empty string returned, no `<style>` injected).  No new
API endpoints, no config flags, no frontend changes.

After `ThemeProvider` mounts and `applyTheme()` writes the same
variables as inline styles on `documentElement`, the values match what
the bootstrap block set, so there is no second-paint discrepancy on the
critical CSS variables.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 31, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Supersedes closed #36000 (same author, same approach). Competes with open #34248 (broader scope: dashboard + TUI flash vs. dashboard-only critical-CSS injection).

@alt-glitch alt-glitch added comp/dashboard Web dashboard / control panel UI (dashboard/, landing) and removed comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for targeting a real first-paint gap: current ThemeProvider still applies defaultTheme before /api/dashboard/themes supplies user definitions and the configured active theme (web/src/themes/context.tsx:451-471, :473-503).

Problems

  • The helper's token set is stale. Current theme application writes --background / --background-base, --midground / --midground-base, --theme-font-sans, and --theme-base-size (web/src/themes/context.tsx:68-102, :375-385), while this patch emits --color-background, --color-midground, --font-sans, and --font-base-size. The direct html, body rule can color the blank canvas, but the claimed parity with applyTheme() is not present on current main.
  • Please add a regression test for user-theme output, built-in no-op behavior, and injection ordering. Existing backend coverage currently stops at normalization/discovery (tests/hermes_cli/test_web_server.py:5692-5736).

Suggested changes

  • Update the bootstrap to emit the current canonical palette and typography tokens, then test the served-index path.
  • The member discussion identifies fix: prevent dashboard and TUI startup theme flash #34248 as the broader competing dashboard/TUI flash solution; reconcile the dashboard bootstrap contract with that work.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Good diagnosis — the first-paint flash for user YAML themes is real (built-ins are bundled in presets.ts, user themes only apply after the /api/dashboard/themes round-trip), and injecting a critical-CSS block from _serve_index() is the right general shape. Three things to fix:

  1. 4 of the 6 injected CSS variables don't exist in the bundle: --color-background, --color-midground, --font-base-size appear nowhere in web/src/, and applyTheme sets --theme-font-sans, not --font-sans. Only --background-base/--midground-base match layerVars(). The visible fix works because the html,body rule uses literal values, but the "ThemeProvider writes the same variables so there's no second-paint discrepancy" claim doesn't hold for half the vars. Please use the real variable names (--theme-font-sans, --theme-base-size, …).
  2. Stale rule on live theme switch: the injected static html,body { background-color; font-family; font-size } block outranks index.css's html { font-family: var(--theme-font-sans) } (same specificity, later in DOM) and is never removed — switching themes in the picker leaves the old theme's font/canvas on html/body until reload. Either have ThemeProvider remove/overwrite #hermes-theme-bootstrap on theme apply, or express the injected rule via the same CSS variables so runtime updates flow through.
  3. Add the snapshot tests you offered.

With those addressed this is a nice UX win — happy to re-review quickly.

teknium1 added a commit that referenced this pull request Jul 16, 2026
…le flows through vars

Review fixes for the inline critical-CSS bootstrap (PR #36024):

1. Variable names now match what the bundle actually consumes.
   --color-background, --color-midground, --font-sans and
   --font-base-size appear nowhere in web/src; the real tokens are:
     --background-base / --midground-base  (layerVars(), context.tsx)
     --theme-font-sans / --theme-base-size (typographyVars(), and
       index.css html{font-family:var(--theme-font-sans);
       font-size:var(--theme-base-size)})

2. Stale-rule bug: the injected html,body rule previously baked in
   literal hex/font values. Because the <style> block sits after the
   bundle's <link> at equal specificity and is never removed, switching
   themes in the picker left the old canvas/font until reload. The rule
   now references the same CSS variables instead of literals —
   applyTheme() writes those vars as inline styles on documentElement,
   which outrank this block in the cascade, so runtime theme switches
   re-resolve the rule automatically. No frontend change needed.
teknium1 added a commit that referenced this pull request Jul 16, 2026
…ction

Server-side coverage for the critical-CSS shim (PR #36024 salvage):

- user theme → style block emitted with ONLY real bundle variable names
  (--background-base/--midground-base from layerVars(),
  --theme-font-sans/--theme-base-size from typographyVars()/index.css),
  and an html,body rule expressed via those vars so runtime theme
  switches never leave a stale canvas/font
- built-in / unknown / non-string active theme → no block
- malformed theme YAML and load_config() exceptions → no crash, index
  still serves
- </style> breakout attempt in a theme value stays escaped
- mount_spa integration: block present in <head> for user themes,
  absent for built-ins
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #65695 (rebase-merged onto main, head commit 7edaaf4) — your commit was cherry-picked with authorship preserved; on top we switched the injected block to the real bundle variables (--theme-font-sans / --theme-base-size — the --color-* names didn't exist in web/src) and made the html,body rule var-driven so live theme switches re-resolve with nothing stale. Thanks for the contribution!

@teknium1 teknium1 closed this Jul 16, 2026
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…le flows through vars

Review fixes for the inline critical-CSS bootstrap (PR NousResearch#36024):

1. Variable names now match what the bundle actually consumes.
   --color-background, --color-midground, --font-sans and
   --font-base-size appear nowhere in web/src; the real tokens are:
     --background-base / --midground-base  (layerVars(), context.tsx)
     --theme-font-sans / --theme-base-size (typographyVars(), and
       index.css html{font-family:var(--theme-font-sans);
       font-size:var(--theme-base-size)})

2. Stale-rule bug: the injected html,body rule previously baked in
   literal hex/font values. Because the <style> block sits after the
   bundle's <link> at equal specificity and is never removed, switching
   themes in the picker left the old canvas/font until reload. The rule
   now references the same CSS variables instead of literals —
   applyTheme() writes those vars as inline styles on documentElement,
   which outrank this block in the cascade, so runtime theme switches
   re-resolve the rule automatically. No frontend change needed.
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…ction

Server-side coverage for the critical-CSS shim (PR NousResearch#36024 salvage):

- user theme → style block emitted with ONLY real bundle variable names
  (--background-base/--midground-base from layerVars(),
  --theme-font-sans/--theme-base-size from typographyVars()/index.css),
  and an html,body rule expressed via those vars so runtime theme
  switches never leave a stale canvas/font
- built-in / unknown / non-string active theme → no block
- malformed theme YAML and load_config() exceptions → no crash, index
  still serves
- </style> breakout attempt in a theme value stays escaped
- mount_spa integration: block present in <head> for user themes,
  absent for built-ins
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…le flows through vars

Review fixes for the inline critical-CSS bootstrap (PR NousResearch#36024):

1. Variable names now match what the bundle actually consumes.
   --color-background, --color-midground, --font-sans and
   --font-base-size appear nowhere in web/src; the real tokens are:
     --background-base / --midground-base  (layerVars(), context.tsx)
     --theme-font-sans / --theme-base-size (typographyVars(), and
       index.css html{font-family:var(--theme-font-sans);
       font-size:var(--theme-base-size)})

2. Stale-rule bug: the injected html,body rule previously baked in
   literal hex/font values. Because the <style> block sits after the
   bundle's <link> at equal specificity and is never removed, switching
   themes in the picker left the old canvas/font until reload. The rule
   now references the same CSS variables instead of literals —
   applyTheme() writes those vars as inline styles on documentElement,
   which outrank this block in the cascade, so runtime theme switches
   re-resolve the rule automatically. No frontend change needed.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…ction

Server-side coverage for the critical-CSS shim (PR NousResearch#36024 salvage):

- user theme → style block emitted with ONLY real bundle variable names
  (--background-base/--midground-base from layerVars(),
  --theme-font-sans/--theme-base-size from typographyVars()/index.css),
  and an html,body rule expressed via those vars so runtime theme
  switches never leave a stale canvas/font
- built-in / unknown / non-string active theme → no block
- malformed theme YAML and load_config() exceptions → no crash, index
  still serves
- </style> breakout attempt in a theme value stays escaped
- mount_spa integration: block present in <head> for user themes,
  absent for built-ins
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants