Added dark mode cuz your app is extremely nice (I use it for myself) …#5
Added dark mode cuz your app is extremely nice (I use it for myself) …#5msjabata25 wants to merge 6 commits into
Conversation
…but the lack of a dark mode (and my allergy to light mode) is an ick i needed to fix. Fix cta buttons, quiz/flashcards feedback colors, added chat markdown support (small change with md suppport but honestly very nice to have). Also added detection for if the user already has dark mode in general or not - Add --button-primary-bg CSS var so CTA buttons stay dark in both themes - Add --feedback-correct/wrong CSS vars for adaptive green/rose indicators - Fix all CTA buttons (Generate/Reveal/Next/Send/Start) to use --button-primary-bg - Fix quiz option feedback, VerdictBadge, DeckComplete, FeedbackCard colors - Darken flashcard rating tones (50→200, 200→400) for dark mode contrast - Fix chat user bubble background in dark mode - Add react-markdown rendering to chat Bubble component
There was a problem hiding this comment.
Code Review
This pull request implements a dark mode theme across the application by introducing a ThemeProvider, updating global CSS variables, and replacing hardcoded colors with theme-aware design tokens. It also adds a dark mode toggle in the settings panel and integrates markdown rendering in the chat view. The feedback highlights a critical bug in the settings API where updating other preferences wipes out the saved theme, and suggests loading the theme server-side to prevent a Flash of Unstyled Content (FOUC). Additionally, recommendations are made to use Tailwind's dark: modifier for flashcard rating colors and to replace hardcoded code block backgrounds with design system tokens to ensure proper contrast in dark mode.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
8 issues found across 18 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
|
@msjabata25 thanks for your submission. |
|
@greptile |
…rk mode gaps - lib/viz-runtime.ts: block sandbox escape via Function constructor chain - lib/work-context.ts, lib/kg.ts, lib/store.ts: atomic write (tmp+rename) - lib/kg-runner.ts: fix TOCTOU race, catch unhandled promise rejection - lib/paths.ts: add docId validation regex to prevent path traversal - electron/main.js: fix dark-mode white flash on startup - app/api/settings/route.ts: preserve theme on partial settings POST - app/layout.tsx + ThemeProvider.tsx: SSR dark class via server-side settings read - components/SettingsButton.tsx: remove hydration guard that dropped toggles - components/RightPane/ChatView.tsx: block img, hardcoded colors → CSS vars, href sanitize - components/RightPane/FeynmanView.tsx: gradient/rose → surface/feedback tokens - components/RightPane/KnowledgeGraphView.tsx: deduplicate API fetch - components/RightPane/VoiceBlob.tsx: add maxWidth:100% for responsive sizing - components/Visualizer/GraphView.tsx: canvas/axis colors adapt to html.dark - components/Visualizer/ThreeDView.tsx: clearColor adapts to html.dark - components/PdfViewer.tsx: bg-white → surface tokens - components/UploadCard.tsx: unmount race guard, feedback tokens, keyboard handler - components/CodexHealthBanner.tsx: rose/amber → feedback/tag tokens - app/globals.css: refine dark palette surface/ink/border/accents
msjabata25
left a comment
There was a problem hiding this comment.
Fixed the issues that were in the ai reviews.
All details in the commit message .
And don't worry I did update the voice blob and made it match the theme
|
@7Mik I have started the AI code review. It will take a few minutes to complete. |
|
The review has been posted above. Here's a quick summary of the key findings: High priority:
Lower priority:
The PR is solid overall — the token design in Tip: You can customize Greptile's behavior for this repo with |
There was a problem hiding this comment.
11 issues found across 29 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
- lib/viz-runtime.ts: null AsyncFunction/GeneratorFunction constructors - lib/work-context.ts, kg.ts, store.ts: use unique tmp suffix per write - lib/paths.ts: widen docId regex to accept non-hex IDs (sample-anatomy) - components/Visualizer/GraphView.tsx: add MutationObserver for theme changes; use ink/inkMuted for grid, axes, legend, axis labels - components/ThemeProvider.tsx: always reapply() on system pref change - components/SettingsButton.tsx: guard stale fetch from overwriting user theme toggle (userToggledRef) - components/RightPane/ChatView.tsx: unconditional whitespace-pre-wrap so non-pulsing bubbles preserve newlines - components/RightPane/FlashcardsView.tsx: hardcoded Tailwind shades → --feedback-wrong/tag-amber/--feedback-correct/tag-sky tokens - components/RightPane/VoiceBlob.tsx: add aspectRatio:1 to prevent canvas distortion when maxWidth clamps width - components/AccountButton.tsx: logout hover uses feedback-wrong tokens - components/WelcomePopup.tsx + globals.css: add --shadow-modal token - components/UploadCard.tsx: add e.preventDefault() on Space/Enter
|
Fixed the remaining issues (the ai reviewer is pointing out already solved issues) so if it comes out with another similar review report then i suggest you review the codebase with something else to avoid fixing the same issue over and over again |
There was a problem hiding this comment.
4 issues found across 29 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…-runtime cleanup, kg-runner return path
- lib/settings-store.ts: unique tmp suffix via randomUUID (prevents write race)
- lib/viz-runtime.ts: hoist constructor refs to module scope; document threat model
- lib/kg-runner.ts: startBuild returns errored KG instead of re-throwing (no
unhandled rejection risk from API route)
- components/Visualizer/ThreeDView.tsx: add MutationObserver for html.dark class
to update renderer clearColor without WebGL teardown; error toast + badge use
CSS variables for theme adaptation
- components/Visualizer/TwoDAnimView.tsx: error toast uses --feedback-wrong-* tokens
- components/RightPane/FlashcardsView.tsx: StatusDot uses --feedback-correct-icon
- components/RightPane/ChatView.tsx: destructure node from code props (internal
react-markdown AST node no longer spreads onto DOM <code>)
- components/SettingsButton.tsx: add osPrefersDark state + matchMedia listener for
reactive effectiveTheme computation
- components/ThemeProvider.tsx: fetch failure no-op (catch(() => {})) so current
DOM class is authoritative on network error
- components/Visualizer/GraphView.tsx: skipResetRef prevents error/reportedRef reset
on theme-only redraws so onRuntimeError isn't re-emitted
There was a problem hiding this comment.
1 issue found across 30 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…model code closure
The model code runs inside an IIFE nested in the `new Function` body. That
IIFE captures the outer function's parameter scope. Previously,
__origFunc/__origAsync/__origGen (the original Function/AsyncFunction/
GeneratorFunction constructors) were passed as parameters to new Function,
making them directly accessible from the model code via closure. The model
code could call __origFunc("return eval")() to regain eval in global scope.
Fix: move constructor nulling/restoring to the wrapper returned by compileFn,
outside the new Function body entirely. The original constructor references
are captured at module scope and live only in the wrapper's closure — the
new Function body (and thus the model code) never sees them.
Without the guard, opening the settings panel and immediately toggling autoGenerate or maxRetries before the /api/settings fetch resolves persists a value derived from the build-time default (AUTO_GENERATE_VIZ / MAX_VIZ_GEN_RETRIES), silently overwriting whatever was saved on disk. The guard was previously removed to allow the theme toggle to work before hydration. Fix: add it back with a skipHydrationCheck parameter, passing true only from onThemeToggle.
|
Cubic reviews and greptile are all clean I think. Everything seems to be working now. I can safely say dark mode and md support for chat are done. |
Added dark mode cuz your app is extremely nice (I use it for myself) but the lack of a dark mode (and my allergy to light mode) is an ick i needed to fix.
Fix cta buttons, quiz/flashcards feedback colors, added chat markdown support (small change with md suppport but honestly very nice to have). Also added detection for if the user already has dark mode in general or not
Added --button-primary-bg CSS var so CTA buttons stay dark in both themes
Added --feedback-correct/wrong CSS vars for adaptive green/rose indicators
Fixed all CTA buttons (Generate/Reveal/Next/Send/Start) to use --button-primary-bg
Fixed quiz option feedback, VerdictBadge, DeckComplete, FeedbackCard colors
Darkened flashcard rating tones (50→200, 200→400) for dark mode contrast
Fixed chat user bubble background in dark mode
Added react-markdown rendering to chat Bubble component
Note: The Feynman session where the bubble thing is is i think still light mode but i'll try and fix it in a later contribution
Summary by cubic
Adds a system-aware dark mode with SSR application and a Settings toggle, with live theme changes across the app. Also hardens the visualizer sandbox and fixes settings persistence races.
New Features
html.darktokens andThemeProvider; follows saved or system preference, reacts tomatchMediaandSETTINGS_EVENT, and prevents FOUC with SSR + inline script.react-markdown, preserve newlines, block images, and sanitizehttps?://links.Bug Fixes
theme; restored hydration guard to prevent stale overwrites (theme toggle is exempt for instant apply);ThemeProviderno-ops on fetch failures; listens to OS preference changes.evalremains shadowed; threat model documented.docIdvalidation (regex widened to allow hyphens).Written for commit ca54643. Summary will update on new commits.
Greptile Summary
This PR adds a system-aware dark mode to the app, including SSR theme injection to prevent FOUC, a
ThemeProviderfor live theme reactivity, and a settings toggle with system-preference detection. It also backfills theme tokens across all UI components, adds Markdown rendering to chat bubbles, and includes several reliability improvements (atomic file writes,docIdvalidation, sandbox hardening inviz-runtime, KG builder returning errored state instead of throwing).html.dark; a blocking inline script handles the system-preference fallback before first paint;ThemeProviderreacts toSETTINGS_EVENTandmatchMediachanges without async fetch on mount.lib/viz-runtime.ts):ORIG_FUNC/ORIG_ASYNC/ORIG_GENare captured at module scope (outsidenew Functionreach) and constructors are nulled in a wrapper closure before model code runs, then restored infinally— correctly closes the closure-escape vector flagged in the previous review.settings-store,kg,store, andwork-context;docIdregex validation on every path helper; KG build returns an errored graph state instead of rejecting to prevent unhandled rejections.Confidence Score: 5/5
Safe to merge — the dark-mode implementation is thorough and the reliability fixes are sound. The three flagged items are minor cosmetic regressions.
The SSR + inline-script FOUC prevention is correctly implemented, the sandbox closure-escape fix is structurally sound (module-scope constants are genuinely unreachable from new Function bodies), atomic writes with UUID tmp suffixes are a clear improvement, and the docId validation closes a real path-construction gap. All three open comments are visual/UX nits: the Electron background flash only affects pre-render milliseconds for light-mode users, the amber error banner loss is cosmetic, and the missing follows-system reset path is a feature gap rather than broken behaviour.
electron/main.js (hardcoded dark titleBarOverlay/backgroundColor) and components/SettingsButton.tsx (no UI path back to system preference) are worth a second look before merge.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Server as Next.js Server participant Script as Inline Script participant HTML as html element participant TP as ThemeProvider participant SB as SettingsButton Server->>HTML: Apply html.dark class via SSR Server->>Script: Embed inline script with saved theme Script->>HTML: Add/remove .dark before first paint Note over Script,HTML: No FOUC — runs synchronously before CSS TP->>HTML: Listen to SETTINGS_EVENT TP->>HTML: Listen to matchMedia change SB->>SB: User opens settings panel SB->>Server: GET /api/settings to hydrate saved theme SB->>SB: User clicks dark mode toggle SB->>HTML: classList.toggle dark SB->>Server: POST /api/settings with theme SB-->>TP: Dispatch SETTINGS_EVENT TP->>HTML: applyTheme from event detail%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Server as Next.js Server participant Script as Inline Script participant HTML as html element participant TP as ThemeProvider participant SB as SettingsButton Server->>HTML: Apply html.dark class via SSR Server->>Script: Embed inline script with saved theme Script->>HTML: Add/remove .dark before first paint Note over Script,HTML: No FOUC — runs synchronously before CSS TP->>HTML: Listen to SETTINGS_EVENT TP->>HTML: Listen to matchMedia change SB->>SB: User opens settings panel SB->>Server: GET /api/settings to hydrate saved theme SB->>SB: User clicks dark mode toggle SB->>HTML: classList.toggle dark SB->>Server: POST /api/settings with theme SB-->>TP: Dispatch SETTINGS_EVENT TP->>HTML: applyTheme from event detailComments Outside Diff (1)
components/RightPane/ChatView.tsx, line 439-451 (link)isInlineis derived from!className. This works for labelled fences (```ts) because react-markdown injectslanguage-ts. For an unlabelled fence (```with no language), react-markdown passes noclassName, soisInlineistrueand the content is wrapped in the small inline<code>span instead of the styled<pre>block. LLM responses frequently omit language labels, so multi-line code will collapse into a single inline chip. A safer guard is to additionally check that the content contains a newline before treating it as inline.Reviews (5): Last reviewed commit: "fix: re-add hydration guard to persist()..." | Re-trigger Greptile