Add Claude Sonnet 5 model and theming system - #3912
Conversation
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sonnet 5 (claude-sonnet-5) shipped in Claude Code 2.1.197 as the new default Sonnet, with native xhigh/max effort and a 1M-token context window. - Register claude-sonnet-5 in BUILT_IN_MODELS with the low/medium/high (default)/xhigh/max effort selector (plus ultracode/ultrathink, matching its closest xhigh-native peer Opus 4.8) and a context-window option that defaults to 1M. - Gate it behind Claude Code 2.1.197 via MINIMUM_CLAUDE_SONNET_5_VERSION / supportsClaudeSonnet5, wired into getBuiltInClaudeModelsForVersion and the version-upgrade-message chain (now newest-first). - Preserve xhigh (don't downgrade to max) for claude-sonnet-5 in normalizeClaudeCliEffort. - Add sonnet-5 / claude-sonnet-5.0 aliases and repoint the bare "sonnet" alias to claude-sonnet-5. Leaves DEFAULT_MODEL_BY_PROVIDER on Sonnet 4.6. - Highlight claude-sonnet-5 with the "NEW" chip in the picker. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Blacksmith runners aren't available on personal accounts, so CI/release workflows couldn't run on this fork. Map each Blacksmith label to the equivalent GitHub-hosted runner, preserving the OS/version and dropping the Blacksmith-specific vCPU sizing: blacksmith-*-ubuntu-2404 -> ubuntu-24.04 blacksmith-12vcpu-macos-26 -> macos-26 blacksmith-32vcpu-windows-2025 -> windows-2025 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
|
Sorry, this was meant for my private fork, this shouldn't have been opened here |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 5 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 69a39c9. Configure here.
| const isDark = theme === "dark" || (theme === "system" && systemDark); | ||
| document.documentElement.classList.toggle("dark", isDark); | ||
| lastAppliedTheme = { theme, systemDark }; | ||
| restoreActiveThemes(); |
There was a problem hiding this comment.
Palette changes skip CSS apply
High Severity
When setActiveLightTheme or setActiveDarkTheme are called, applyTheme is triggered but returns early because only the active palette ID changes, not the main theme or system dark mode. This prevents restoreActiveThemes from running, so the app's CSS custom properties for the new palette are not applied until a color mode change or page reload.
Reviewed by Cursor Bugbot for commit 69a39c9. Configure here.
| if (source.id === DEFAULT_THEME_ID) return copy; | ||
| if (source.light) copy.light = materializeTokens(source.light); | ||
| if (source.dark) copy.dark = materializeTokens(source.dark); | ||
| return copy; |
There was a problem hiding this comment.
Syntax theme lost on share
Medium Severity
The serializeTheme, parseTheme, and duplicateTheme functions don't handle the syntax field. This means themes with custom Shiki mappings lose them when exported, imported, or duplicated, causing code blocks and diffs to fall back to a default highlighting theme.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 69a39c9. Configure here.
| } | ||
|
|
||
| export function GeneralSettingsPanel() { | ||
| const { theme, setTheme } = useTheme(); |
There was a problem hiding this comment.
Appearance restore ignores palettes
Medium Severity
The "Restore defaults" button on the Appearance settings page doesn't fully reset theme settings. It only tracks and resets the overall color mode (theme), but ignores specific light/dark theme palettes (activeLightThemeId, activeDarkThemeId). This can lead to the button being disabled when only palettes are changed, or failing to reset custom palettes when activated.
Reviewed by Cursor Bugbot for commit 69a39c9. Configure here.
| const style = ensureThemeStyleElement(); | ||
| const lightCss = tokensToCss(materializeTokens(resolveTokens(lightTheme, "light"))); | ||
| const darkCss = tokensToCss(materializeTokens(resolveTokens(darkTheme, "dark"))); | ||
| style.textContent = `:root { ${lightCss} } :root.dark { ${darkCss} }`; |
There was a problem hiding this comment.
Theme CSS allow-list bypassed
Medium Severity
tokensToCss / applyActiveThemes emit every string entry from resolved token maps, and save/import never strip to THEME_TOKEN_NAMES. The editor claims unknown tokens are ignored, and isValidColorValue still allows ; / }, so imported or edited theme JSON can break out of the injected <style> tag and inject arbitrary CSS.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 69a39c9. Configure here.
| const startEditing = useCallback((target: ThemeDefinition) => { | ||
| setEditingId(target.id); | ||
| setDraft(target); | ||
| }, []); |
There was a problem hiding this comment.
Edit switch drops unsaved draft
Low Severity
startEditing replaces editingId and draft immediately. Clicking Edit on another theme while the current editor has unsaved changes discards that draft with no confirmation, even though Cancel uses a discard dialog for the same risk.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 69a39c9. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| return BUILT_IN_THEMES.some((theme) => theme.id === id); | ||
| } | ||
|
|
||
| export function isValidColorValue(value: string): boolean { |
There was a problem hiding this comment.
🟠 High themes/registry.ts:19
isValidColorValue only rejects a few substrings like javascript: and <script, so values containing CSS delimiters such as red; } body { display: none } :root { pass validation. tokensToCss then interpolates these values verbatim into a <style> element, so importing or JSON-editing a theme with such a value injects arbitrary stylesheet rules that can hide or alter the entire application UI instead of being constrained to a single token value. Consider rejecting any value containing ;, {, }, or < so only a single CSS custom-property value can be produced.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/themes/registry.ts around line 19:
`isValidColorValue` only rejects a few substrings like `javascript:` and `<script`, so values containing CSS delimiters such as `red; } body { display: none } :root {` pass validation. `tokensToCss` then interpolates these values verbatim into a `<style>` element, so importing or JSON-editing a theme with such a value injects arbitrary stylesheet rules that can hide or alter the entire application UI instead of being constrained to a single token value. Consider rejecting any value containing `;`, `{`, `}`, or `<` so only a single CSS custom-property value can be produced.
| sonnet: "claude-sonnet-5", | ||
| "sonnet-5": "claude-sonnet-5", |
There was a problem hiding this comment.
🟠 High src/model.ts:177
Changing the sonnet alias to claude-sonnet-5 makes resolveSelectableModel(..., "sonnet", options) return null on Claude Code versions below 2.1.197: those versions filter Sonnet 5 out of options, and the resolver only resolves an alias whose target is present in options. Previously sonnet resolved to claude-sonnet-4-6, so users on supported-but-older Claude Code versions can no longer resolve the generic sonnet selection even though Sonnet 4.6 remains available. Consider restoring the sonnet alias to claude-sonnet-4-6, or gating it by Claude Code version so the shorthand still resolves for older clients.
- sonnet: "claude-sonnet-5",
+ sonnet: "claude-sonnet-4-6",
"sonnet-5": "claude-sonnet-5",🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/contracts/src/model.ts around lines 177-178:
Changing the `sonnet` alias to `claude-sonnet-5` makes `resolveSelectableModel(..., "sonnet", options)` return `null` on Claude Code versions below `2.1.197`: those versions filter Sonnet 5 out of `options`, and the resolver only resolves an alias whose target is present in `options`. Previously `sonnet` resolved to `claude-sonnet-4-6`, so users on supported-but-older Claude Code versions can no longer resolve the generic `sonnet` selection even though Sonnet 4.6 remains available. Consider restoring the `sonnet` alias to `claude-sonnet-4-6`, or gating it by Claude Code version so the shorthand still resolves for older clients.
| const editing = | ||
| editingId !== null && draft !== null ? { source: findTheme(editingId), draft } : null; |
There was a problem hiding this comment.
🟡 Medium settings/AppearanceSettings.tsx:79
Typing in a token field immediately discards the live preview and reverts the page to the persisted theme. The editing.source value is rebuilt via findTheme(editingId) on every render, which reparses local storage and returns a new object each time. ThemeEditor's cleanup effect is keyed on [source], so every keystroke triggers the cleanup, which calls restoreActiveThemes() and overwrites the in-progress preview that the preview effect just applied. Memoize editing.source so it stays referentially stable while the same theme is being edited.
| const editing = | |
| editingId !== null && draft !== null ? { source: findTheme(editingId), draft } : null; | |
| const editingSource = useMemo(() => editingId !== null ? findTheme(editingId) : null, [editingId, themes]); | |
| const editing = | |
| editingId !== null && draft !== null && editingSource !== null ? { source: editingSource, draft } : null; |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/AppearanceSettings.tsx around lines 79-80:
Typing in a token field immediately discards the live preview and reverts the page to the persisted theme. The `editing.source` value is rebuilt via `findTheme(editingId)` on every render, which reparses local storage and returns a new object each time. `ThemeEditor`'s cleanup effect is keyed on `[source]`, so every keystroke triggers the cleanup, which calls `restoreActiveThemes()` and overwrites the in-progress preview that the preview effect just applied. Memoize `editing.source` so it stays referentially stable while the same theme is being edited.
| emitChange(); | ||
| }, []); | ||
|
|
||
| const setActiveLightTheme = useCallback((id: string) => { |
There was a problem hiding this comment.
🟡 Medium hooks/useTheme.ts:350
setActiveLightTheme and setActiveDarkTheme call setActiveLightThemeId/setActiveDarkThemeId directly without catching errors, unlike setTheme which wraps writeThemePreference in try/catch. If localStorage.setItem throws (quota or security rejection), the exception propagates out of the React event handler — no theme change is applied and no storage error is logged. Consider wrapping these calls in the same error handling used by setTheme, throwing a ThemeStorageError and logging it.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/hooks/useTheme.ts around line 350:
`setActiveLightTheme` and `setActiveDarkTheme` call `setActiveLightThemeId`/`setActiveDarkThemeId` directly without catching errors, unlike `setTheme` which wraps `writeThemePreference` in try/catch. If `localStorage.setItem` throws (quota or security rejection), the exception propagates out of the React event handler — no theme change is applied and no storage error is logged. Consider wrapping these calls in the same error handling used by `setTheme`, throwing a `ThemeStorageError` and logging it.
| export function serializeTheme(theme: ThemeDefinition): string { | ||
| const payload: ThemeDefinition = { id: theme.id, name: theme.name }; | ||
| if (theme.description) payload.description = theme.description; | ||
| if (theme.light) payload.light = { ...theme.light }; | ||
| if (theme.dark) payload.dark = { ...theme.dark }; | ||
| return JSON.stringify(payload, null, 2); | ||
| } |
There was a problem hiding this comment.
🟡 Medium themes/transport.ts:27
serializeTheme omits the syntax field from the serialized payload, so exporting a theme and then importing it silently drops the Shiki syntax-highlighting configuration. Themes like Solarized, Nord, Catppuccin, and Rosé Pine define palette-specific syntax themes, but after a round-trip through serializeTheme/parseTheme that data is lost and they fall back to the generic syntax theme. The function copies description, light, and dark but never copies syntax. Consider including theme.syntax in the payload when present.
export function serializeTheme(theme: ThemeDefinition): string {
const payload: ThemeDefinition = { id: theme.id, name: theme.name };
if (theme.description) payload.description = theme.description;
if (theme.light) payload.light = { ...theme.light };
if (theme.dark) payload.dark = { ...theme.dark };
+ if (theme.syntax) payload.syntax = { ...theme.syntax };
return JSON.stringify(payload, null, 2);
}🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/themes/transport.ts around lines 27-33:
`serializeTheme` omits the `syntax` field from the serialized payload, so exporting a theme and then importing it silently drops the Shiki syntax-highlighting configuration. Themes like Solarized, Nord, Catppuccin, and Rosé Pine define palette-specific `syntax` themes, but after a round-trip through `serializeTheme`/`parseTheme` that data is lost and they fall back to the generic syntax theme. The function copies `description`, `light`, and `dark` but never copies `syntax`. Consider including `theme.syntax` in the payload when present.
| const hasInvalidValue = Object.values(variantTokens).some( | ||
| (value) => typeof value === "string" && value.length > 0 && !isValidColorValue(value), | ||
| ); | ||
| const saveDisabled = jsonError !== null || draft.name.trim().length === 0 || hasInvalidValue; |
There was a problem hiding this comment.
🟡 Medium settings/ThemeEditor.tsx:209
The JSON editor allows the user to change draft.id to the id of an existing theme, and saveDisabled never detects that collision. When the user clicks Save with a duplicate id, updateCustomTheme throws Theme id ... already exists and onSave (in AppearanceSettings) does not catch it, so the editor throws an uncaught runtime error instead of staying open with validation feedback. Consider including id-collision detection in saveDisabled so Save is blocked (or shows an inline error) when the new id already belongs to another theme.
| const saveDisabled = jsonError !== null || draft.name.trim().length === 0 || hasInvalidValue; | |
| const saveDisabled = jsonError !== null || draft.name.trim().length === 0 || hasInvalidValue || (draft.id !== source.id && findTheme(draft.id) != null); |
Also found in 1 other location(s)
apps/web/src/components/settings/AppearanceSettings.tsx:103
handleSavecallsupdateCustomThemewithout handling its duplicate-ID error. In the exposed JSON editor, a user can change a custom theme'sidto any existing built-in or custom ID; the draft remains schema-valid and Save stays enabled, but clicking Save throws fromupdateCustomThemewith no toast or validation feedback, leaving the save action broken.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/ThemeEditor.tsx around line 209:
The JSON editor allows the user to change `draft.id` to the id of an existing theme, and `saveDisabled` never detects that collision. When the user clicks Save with a duplicate id, `updateCustomTheme` throws `Theme id ... already exists` and `onSave` (in `AppearanceSettings`) does not catch it, so the editor throws an uncaught runtime error instead of staying open with validation feedback. Consider including id-collision detection in `saveDisabled` so Save is blocked (or shows an inline error) when the new id already belongs to another theme.
Also found in 1 other location(s):
- apps/web/src/components/settings/AppearanceSettings.tsx:103 -- `handleSave` calls `updateCustomTheme` without handling its duplicate-ID error. In the exposed JSON editor, a user can change a custom theme's `id` to any existing built-in or custom ID; the draft remains schema-valid and Save stays enabled, but clicking Save throws from `updateCustomTheme` with no toast or validation feedback, leaving the save action broken.
| { cause: error }, | ||
| ); | ||
| } | ||
| if (!isValidTheme(parsed)) { |
There was a problem hiding this comment.
🟡 Medium themes/transport.ts:45
parseTheme accepts a theme with a non-string description (e.g. {}) because isValidTheme never validates the type of description. The truthy object is stored on the theme at next.description = parsed.description, and when the settings page later renders entry.description as a React child, it throws on object values instead of rejecting the malformed import. Add a typeof candidate.description === "string" check (or otherwise guard description) before trusting it.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/themes/transport.ts around line 45:
`parseTheme` accepts a theme with a non-string `description` (e.g. `{}`) because `isValidTheme` never validates the type of `description`. The truthy object is stored on the theme at `next.description = parsed.description`, and when the settings page later renders `entry.description` as a React child, it throws on object values instead of rejecting the malformed import. Add a `typeof candidate.description === "string"` check (or otherwise guard `description`) before trusting it.
| const canGoBack = useCanGoBack(); | ||
| const [restoreSignal, setRestoreSignal] = useState(0); | ||
| const showRestoreDefaults = location.pathname === "/settings/general"; | ||
| const showRestoreDefaults = |
There was a problem hiding this comment.
🟡 Medium routes/settings.tsx:40
The RestoreDefaultsButton shown on /settings/appearance is never enabled when the user only changes light/dark palette selections, and when enabled for other dirty settings it still leaves palette selections unrestored. useSettingsRestore only tracks theme and legacy general settings as dirty and only resets those values; activeLightThemeId and activeDarkThemeId are neither checked nor reset. As a result, changing a palette while leaving color mode unchanged leaves the button disabled, and restoring defaults from the Appearance page keeps the non-default palettes applied. Consider including activeLightThemeId and activeDarkThemeId in both the dirty check and the restore logic in useSettingsRestore.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/routes/settings.tsx around line 40:
The `RestoreDefaultsButton` shown on `/settings/appearance` is never enabled when the user only changes light/dark palette selections, and when enabled for other dirty settings it still leaves palette selections unrestored. `useSettingsRestore` only tracks `theme` and legacy general settings as dirty and only resets those values; `activeLightThemeId` and `activeDarkThemeId` are neither checked nor reset. As a result, changing a palette while leaving color mode unchanged leaves the button disabled, and restoring defaults from the Appearance page keeps the non-default palettes applied. Consider including `activeLightThemeId` and `activeDarkThemeId` in both the dirty check and the restore logic in `useSettingsRestore`.
| const isDark = theme === "dark" || (theme === "system" && systemDark); | ||
| document.documentElement.classList.toggle("dark", isDark); | ||
| lastAppliedTheme = { theme, systemDark }; | ||
| restoreActiveThemes(); |
There was a problem hiding this comment.
🟠 High hooks/useTheme.ts:206
Calling restoreActiveThemes() inside applyTheme reintroduces an unguarded localStorage read: restoreActiveThemes calls getActiveLightThemeId/getActiveDarkThemeId, which read storage without try/catch. When storage access throws (e.g., disabled cookies/private mode — the exact case getStored()/readThemePreference already handle), applyTheme throws instead of falling back to the default theme, so the module-load applyTheme(getStored()) call crashes and the theme hook fails to initialize. Wrap the restoreActiveThemes() call in a try/catch (or guard it in the registry) so storage errors fall back gracefully like the rest of this module.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/hooks/useTheme.ts around line 206:
Calling `restoreActiveThemes()` inside `applyTheme` reintroduces an unguarded `localStorage` read: `restoreActiveThemes` calls `getActiveLightThemeId`/`getActiveDarkThemeId`, which read storage without `try/catch`. When storage access throws (e.g., disabled cookies/private mode — the exact case `getStored()`/`readThemePreference` already handle), `applyTheme` throws instead of falling back to the default theme, so the module-load `applyTheme(getStored())` call crashes and the theme hook fails to initialize. Wrap the `restoreActiveThemes()` call in a `try/catch` (or guard it in the registry) so storage errors fall back gracefully like the rest of this module.
ApprovabilityVerdict: Needs human review 9 blocking correctness issues found. This PR introduces a substantial new theming system feature with ~2500+ lines of new code, plus model alias changes. Multiple unresolved review comments identify HIGH severity issues including potential CSS injection vulnerabilities, localStorage crash paths, and breaking behavior for older Claude Code versions. The scope and open issues warrant human review. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Summary
claude-sonnet-5) as a built-in model with reasoning-effort and 200k/1M context-window options, gated behind Claude Code ≥2.1.197with an upgrade message for older versionsxhigheffort for Sonnet 5 innormalizeClaudeCliEffortand flag it as a "new" model in the model pickerDiffWorkerError.themeNameto any Shiki theme idubuntu-24.04,macos-26,windows-2025)Testing
ClaudeAdapter.test.ts: preservesxhigheffort for Claude Sonnet 5ProviderRegistry.test.ts: shows Sonnet 5 on supported versions, hides it with the upgrade message on older onesThemeEditor.test.ts,themes/registry.test.ts,themes/transport.test.ts,diffRendering.syntaxTheme.test.ts: theming registry/editor/transport/syntax-theme behaviorNote
Medium Risk
Large user-facing theming surface and model-selection changes affect defaults and highlighting; CI runner swap may change build times or capacity but not app runtime security.
Overview
Adds Claude Sonnet 5 as a built-in model (reasoning effort, 200k/1M context) gated on Claude Code ≥ 2.1.197, keeps
xhigheffort for that slug, updates defaultsonnetaliases to Sonnet 5, and marks it NEW in the model picker.Introduces a token-based theming system: built-in palettes (Default, Solarized, Nord, Catppuccin, Rosé Pine, High Contrast), separate light/dark theme slots, a new Appearance settings route with import/export/duplicate/edit, runtime CSS injection with
--alpha→color-mix, and palette-aware Shiki highlighting (highlighter cache keyed by theme).Moves color mode out of General settings into Appearance; scrollbars/noise use theme CSS variables.
CI/release/deploy workflows switch from Blacksmith runners to GitHub-hosted (
ubuntu-24.04,macos-26,windows-2025).Reviewed by Cursor Bugbot for commit 69a39c9. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Claude Sonnet 5 model and a token-based theming system with an Appearance settings page
claude-sonnet-5as a new built-in model (gated on Claude CLI ≥ v2.1.197), preservesxhigheffort for it, and routes thesonnetalias to it instead ofclaude-sonnet-4-6./settings/appearancepage (AppearanceSettingsPanel) for selecting active light/dark themes and color mode, with create/duplicate/edit/export/import/delete of custom themes.ThemeEditorcomponent (ThemeEditor.tsx) with live preview, per-variant token editing, JSON mode with schema validation, and discard confirmation.ChatMarkdownand diff rendering now selects the active palette's Shiki theme id, falling back to Pierre themes.ubuntu-24.04,macos-26,windows-2025).sonnetmodel alias now resolves toclaude-sonnet-5; any caller relying on it resolving toclaude-sonnet-4-6will target a different model.📊 Macroscope summarized 69a39c9. 23 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.