pull request

[GeoffBao]

No description provided.

[nesquena-hermes]

Thanks for the PR, @GeoffBao! Three nice additions here — Claude skin, splash hero redesign, and DeepSeek V4 model entries. A few notes for the maintainer:

Claude skin (feat: add Claude skin with Anthropic-inspired warm palette)

• Clay accent palette (#D97757, #C06A49, #9A523A) and the Tiempos serif headline for the Claude skin are a clean fit for the brand.
• The follow-up fix for new-chat button contrast (clay-on-clay invisible text in light mode) looks like the right specificity lift.

Splash hero redesign (feat(ui): Hermes splash hero + fix Claude skin new-chat contrast)

• CLI-style splash with the 4-up stat grid (skills / sessions / spaces / profiles) is a meaningful UX improvement over the blank empty state.
• Stats load async from /api/skills, /api/sessions, /api/workspaces, /api/profiles with graceful em-dash fallback — good pattern.
• Worth confirming the stat calls don't fire on every page init for users who never see the empty state (e.g. only fetch when the splash is actually rendered).

DeepSeek V4 model entries (fix(models): add DeepSeek V4 flash/pro options to WebUI)

• Straightforward model-list update. Confirm model IDs match the actual DeepSeek API identifiers (e.g. deepseek-chat vs deepseek-v4-pro).

Minor:

• PR title "pull request" is a placeholder — worth updating to something like "feat: Claude skin + splash hero redesign + DeepSeek V4 models" before merge.
• PR body is empty — a brief summary of the three changes would help reviewers.

Overall the direction is solid. Flagging for maintainer review.

[nesquena]

Review — request changes 🟠

Thanks @GeoffBao (and Oz) for putting this together — the Claude skin palette and the splash-hero redesign both look thoughtful, and bringing the WebUI's DeepSeek model list in sync with the upstream agent is a real fix. There are a few things I'd like to land before this can ship as-is, plus a real test regression that the PR introduces.

What this ships

Three substantial changes layered into a single PR:

1. DeepSeek V4 models — adds deepseek/deepseek-v4-flash and deepseek/deepseek-v4-pro to api/config.py:504-505, api/config.py:646-647, static/index.html:394-395, and static/ui.js:641.
2. New "Claude" skin — palette, typography, and per-skin overrides in static/style.css:84-185, plus a registry entry in static/boot.js:622.
3. CLI-style splash hero — full redesign of #emptyState in static/index.html:204-254, plus 90 lines of new CSS in static/style.css:2238-2287 and a new refreshHermesHero() function in static/boot.js:929-977.

Traced against upstream hermes-agent

Pulled a fresh nousresearch/hermes-agent tarball. Good news: deepseek-v4-pro and deepseek-v4-flash are already first-class model IDs upstream — see hermes_cli/models.py:36-37 and hermes_cli/model_normalize.py:134-135. So when a user picks one of these in the WebUI, the agent's normalizer accepts it and the round-trip through config.yaml.model.default is safe. ✅

Concerns

1. 🚫 Silent migration of existing users to the Claude skin

static/index.html:18 contains:

if(!migrated && (rawSkin===null || s==='default')){ skin='claude'; }
localStorage.setItem('hermes-skin-claude-migrated','1');

This force-changes every existing user with the default skin to claude the next time they load the page. Anyone who deliberately chose to stay on the default look will suddenly see a different UI, and the only way to restore their preference is to manually pick "Default" in Settings. New skins should be opt-in via the settings panel — not a silent override.

Same script also flips the default theme from dark to light for first-time visitors ((rawTheme||'light') and the fallbacks at the end of the line). That's also a values change worth its own discussion — please don't bundle it with a skin add.

Recommendation: drop the if(!migrated...) block and the localStorage.setItem('hermes-skin-claude-migrated', '1') write. Restore the 'dark' defaults on the rawTheme/themes[t]?t:'dark' fallbacks. The Claude skin should be available in the picker, not auto-applied.

2. 📦 Three substantial features in one PR

CONTRIBUTING.md (Path 2 / "One Logical Change Per PR") explicitly asks for separation when changes are this size:

"A bug fix plus a CSS cleanup plus a refactor plus a docs rewrite is not [a small related group]."

I'd ask you to split this into three PRs so each can be reviewed on its own merits:

• PR A: DeepSeek V4 model entries (the smallest, lowest-risk change — would land within hours).
• PR B: Claude skin (palette + typography + per-skin overrides). Without the migration block, this is purely additive.
• PR C: Splash hero redesign (HTML + CSS + the refreshHermesHero JS).

Each PR can include its own description, screenshots, and (where applicable) tests.

3. 🧪 Test regression — 5 tests fail on this branch

Running the suite on the PR branch:

FAILED tests/test_sprint20b.py::test_send_btn_is_circle
FAILED tests/test_sprint20b.py::test_send_btn_fixed_dimensions
FAILED tests/test_sprint20b.py::test_send_btn_accent_background
FAILED tests/test_sprint20b.py::test_send_btn_has_transition
FAILED tests/test_sprint20b.py::test_send_btn_hover_has_scale
5 failed, 2389 passed, 47 skipped, 8 subtests passed

Root cause traced: the Claude skin block adds :root[data-skin="claude"] .send-btn{box-shadow:...} at static/style.css:163, which appears earlier in the file than the canonical .send-btn{...} at line 786. The tests use bare css.find('.send-btn{') which returns the first occurrence — now matching only the skin's box-shadow tweak (which lacks width, transition, var(--accent), etc.).

The CSS itself is correct on both sides; the test pattern is the culprit. I prepared a fix that scopes the lookup to the un-prefixed canonical rule via a multiline ^\s*<selector>\{ regex, but couldn't push it to your fork's master from my review sandbox. Please apply this patch in PR B (the Claude skin PR):

--- a/tests/test_sprint20b.py
+++ b/tests/test_sprint20b.py
@@ -13,6 +13,22 @@ def get_text(path):
         return r.read().decode(), r.status


+def _find_canonical_rule(css, selector):
+    """Return the body of the *canonical* (un-skinned) CSS rule for `selector`.
+    Skin overrides like `:root[data-skin="claude"] .send-btn{...}` start with
+    a longer prefix on the same line, so a bare `css.find('.send-btn{')` may
+    return one of those overrides instead of the canonical rule."""
+    pattern = r'^\s*' + re.escape(selector) + r'\{'
+    m = re.search(pattern, css, re.MULTILINE)
+    if not m:
+        return ''
+    brace_open = m.end() - 1
+    brace_close = css.find('}', brace_open)
+    return css[brace_open:brace_close]
+
+
 # ── index.html ────────────────────────────────────────────────────────────

Then in each of the 6 send-btn tests, replace:

    send_idx = css.find('.send-btn{')
    brace_open = css.find('{', send_idx)
    brace_close = css.find('}', brace_open)
    rule = css[brace_open:brace_close]

with:

    rule = _find_canonical_rule(css, '.send-btn')   # or '.send-btn:hover' for that one test

After this fix the full suite is back to 2394 passed, 47 skipped, 0 failed locally.

4. 🔁 refreshHermesHero() runs on every visibilitychange

static/boot.js:976 wires refreshHermesHero() to visibilitychange on the window:

window.addEventListener('visibilitychange',()=>{ if(!document.hidden){ try{ refreshHermesHero(); }catch(_){ } } });

That fires every time the user tabs back to the page, regardless of whether the splash is currently visible — and each call kicks off four parallel API requests (/api/skills, /api/sessions, /api/workspaces, /api/profiles). For someone who's mid-conversation and tabs back to the WebUI, that's 4 unnecessary requests per tab-focus.

Recommendation: gate the refresh on whether #emptyState is actually visible in the viewport — e.g. if (!document.hidden && document.getElementById('emptyState')?.offsetParent !== null). Same gate worth adding to the initial setTimeout(...,400) call.

5. 📝 Empty PR description

CONTRIBUTING.md asks for a Thinking Path / What Changed / Why It Matters in the body. The PR title pull request is a placeholder, and the body is empty. Even if you split into 3 PRs, each one should have a brief description so reviewers don't have to reverse-engineer intent from the diff.

Other audit — things that are correct already

• DeepSeek V4 IDs — verified upstream (hermes_cli/models.py:36-37, hermes_cli/model_normalize.py:134-135). Round-trip through config.yaml.model.default is safe.
• Claude skin palette — pure CSS-variable rewrites, no specificity foot-guns spotted in the typography block; the :root[data-skin="claude"]:not(.dark) .new-chat-btn selector at style.css:159 correctly addresses the clay-on-clay invisible-text bug from earlier in the diff.
• hermes-hero HTML/CSS — semantic markup, ARIA labels, mobile-responsive rules at the bottom. Looks good visually (didn't run it in a browser).
• Security — no XSS risk: stat values are set via el.textContent = String(val) (boot.js:931), not innerHTML. The graceful em-dash fallback on API failure is good defensive UX.

Edge-case trace

Scenario	Expected	Actual
New user (empty localStorage) opens the page	dark + default (the project's documented defaults)	light + claude 🚫
Existing user with default skin	keeps default	force-migrated to claude 🚫
Existing user with slate (or any non-default) skin	keeps their skin	keeps their skin ✅
User picks DeepSeek V4 in dropdown	round-trips through agent	works (verified upstream) ✅
User tabs away from chat then back	no extra network calls	4 API calls fire 🟠
Test suite (full)	0 failed	5 failed 🚫

Recommendation

Request changes. The three big asks are:

1. Drop the silent skin migration block (and the dark→light default flip).
2. Split into 3 PRs (DeepSeek V4 / Claude skin / splash hero). The DeepSeek one can land basically immediately.
3. Apply the test patch above in whichever PR ends up shipping the Claude skin block.

The refreshHermesHero visibility-gate and an explicit PR description would round it out nicely.

Once the migration is removed and the work is split, this is genuinely good direction — the Claude skin is well-executed and the splash hero is a real UX upgrade. Looking forward to landing each piece.

Thanks again for the contribution!

[nesquena-hermes]

Hold — PR needs a description and addressed review feedback

Thanks for the contribution, @GeoffBao! Before this can proceed, two things are needed:

1. PR description — the PR body is currently empty. Please add a description of what the PR changes and why, with screenshots if it touches UI.
2. Address the review feedback — the prior review comment listed specific things to fix (Claude skin color contrast, model entry format, etc.). Please address those before requesting re-review.

Happy to help once both are in place!

[nesquena-hermes]

Salvaged into 4 focused PRs (now all shipped) 🎉

Hi @GeoffBao — thanks for putting this PR together. It bundled multiple distinct changes, so we split it into focused PRs and pulled in the parts that worked well. Here's the salvage map.

Pulled in and shipped ✅

Original part	Shipped as	Release	Status
Warm clay/sand color palette	PR #1307 — Sienna skin (renamed)	v0.50.244	✅ Merged
Stale 404 cleanup + structured api() errors	PR #1304 — fix: clear stale saved session on 404 + structured api() errors	v0.50.244	✅ Merged
Cmd/Ctrl+K works while busy	PR #1306 — fix: allow Cmd/Ctrl+K new chat while a conversation is busy	v0.50.244	✅ Merged

The skin palette was renamed from "Claude" to Sienna — descriptive paint-name that captures the warm clay/sand earth tones without trademark concerns. Other adjustments:

• Sienna is opt-in. No forced migration of existing users. Default skin stays default (gold).
• Default theme stays dark. No silent flip from dark → light.
• hermes-skin-claude-migrated localStorage flag removed (it only existed to support the forced migration, not needed for opt-in).
• Specificity-boosted .new-chat-btn override to prevent clay-on-clay invisible text in light mode (was a real bug in the original).
• 6 regression tests in tests/test_sienna_skin.py lock in the opt-in behavior, allowlist registration, and clay-on-clay guard.

Co-authored-by trailers preserved in all 3 squash commits.

Not pulled in (with reasons)

Original part	Why discarded
Forced default-skin migration on first boot	Silently migrated every existing user to clay without notice. Hostile UX — users woke up with a different theme.
Theme default flip from dark → light	Surprise behavior change; flipped the baseline every existing user expects.
CLI-style "HERMES · AGENT" empty-state hero with 4-up stat tiles	Big UX redesign that fired 4 API calls (/api/skills, /api/sessions, /api/workspaces, /api/profiles) on every visibility change. Significant scope, marginal value. If we want this it should be its own thoughtful PR with proper design discussion — not bundled inside a skin PR.
Session title quality improvements (originally part of this PR)	We did pursue this as PR #1305 but ultimately declined to ship it in this round; we may revisit the heuristics later.
HERMES.md reformatting (244 lines)	Pure Prettier-style markdown table/indent reflow. Zero content change.

Closing this PR

All the salvageable pieces are on master and live in production. Closing this PR — please continue contributing! When PRs combine multiple distinct changes, splitting them into focused PRs (one feature/fix per PR) makes review faster and gives you better feedback per change.

Closes #1084.