Repository navigation
Refactor: Infrastructure cleanup, shared components, and theme synchronization - #75
Merged
Merged
Conversation
…1–2) Consolidates duplicated cross-cutting idioms surfaced in the architecture review (docs/architecture-review.md). Wave 1 — shared hooks/helpers: - getIsDark()/useIsDark() + useThemeRepaint(); migrate 7 data-theme reads and 2 canvas observer blocks (fixes stale-theme on live switch) - useCopyToClipboard(); migrate ~7 copy-with-reset sites - navigateAndReveal()/revealNote/Card/Column; migrate 13 setView+dispatch sites and fix no-timeout race variants - move withAlpha() into analyticsUtils; unify priority/status colour maps (align Tailwind medium hue to --info) Wave 2 — component extractions: - merge card-detail + card-detail-panel into CardDetailBody - single syntax palette (lib/syntax-palette.ts) shared by CodeBlock, editor-theme, dashboard-view, PDF export (byte-identical output) - ui/toggle.tsx primitive; NodeTypeChip shared component - replace inline avatars/cursors with MessageAvatar/StreamingCursor type-check:all + 836 tests + compile all green.
- store: extract restorePersistedTheme/restorePersistedUiPrefs shared by both hydration paths (removes ~40 duplicated lines) - notes: extract PDF-export HTML prep into note-pdf-export.ts - notes: extract shared ReactMarkdown pre→CodeBlock/Mermaid handler into markdown-code-fence.ts; used by note-editor, NoteMarkdownPreview, chat - kanban: extract inline archive view into ArchiveView (board.tsx 657→549) Slice split of terminal-sessions.ts skipped (high risk, no user benefit) and GitView decomposition deferred. type-check:all + 836 tests + compile all green.
…ave 3.5) - §2.8 move sessionUtils.formatDate → lib/utils formatDateCompact; delete sessionUtils.ts - §3.5 extract persistPiTranscript() shared by AgentChatPane onDone/onError - add changelogs/v2.3.6.md; log wave 3.5 in architecture-review.md
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThis PR introduces shared UI primitives (useIsDark, useCopyToClipboard, navigation reveal helpers, Toggle, NodeTypeChip, syntax color palette), refactors numerous components to consume them, unifies card-detail rendering, extracts archive-view/PDF-export/markdown-fence logic into dedicated modules, consolidates store hydration, and adds a changelog plus architecture review document. ChangesShared primitives extraction and consumer refactors
Documentation
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant UI
participant revealNote
participant setView
participant window
UI->>revealNote: click note/card reference
revealNote->>setView: setView("notes"/"board")
revealNote->>window: dispatchEvent CairnEvents (after REVEAL_DELAY_MS)
window-->>UI: target view selects/scrolls to item
sequenceDiagram
participant Component
participant useCopyToClipboard
participant Clipboard
Component->>useCopyToClipboard: copy(text, key?)
useCopyToClipboard->>Clipboard: navigator.clipboard.writeText(text)
Clipboard-->>useCopyToClipboard: success/failure
useCopyToClipboard-->>Component: copied=true, auto-reset after resetMs
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/settings/ViewVisibilitySettings.tsx (1)
42-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDuplicate accessible name for the two "always visible" switches.
label="Always visible"is hardcoded, so both the "Overview" and "Notes" switches get the identicalaria-label. Screen reader users can't tell them apart. The per-rowlabelvariable (used correctly below at Line 73 for the toggleable rows) should be interpolated here too.🛠️ Proposed fix
- <Toggle checked disabled onCheckedChange={() => {}} label="Always visible" /> + <Toggle checked disabled onCheckedChange={() => {}} label={`${label} - Always visible`} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/settings/ViewVisibilitySettings.tsx` around lines 42 - 55, The Toggle in ViewVisibilitySettings is using a hardcoded accessible name, so both rows announce the same label. Update the Toggle inside the ["Overview", "Notes"] map to include the per-row label variable in its accessible label, matching the pattern already used by the toggleable rows elsewhere in the component, so each switch is uniquely identified by its row name.
🧹 Nitpick comments (6)
src/components/settings/shared.tsx (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnify the Toggle entry point This wrapper only forwards
checked/onChange/label/id, whileViewVisibilitySettings.tsximports@/components/ui/toggledirectly fordisabled. Re-export the canonicalTogglehere or forward the fullTogglePropssurface so settings code has one import shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/settings/shared.tsx` at line 6, The shared Toggle wrapper currently exposes only a narrow prop subset, while other settings code imports the UI Toggle directly for additional props like disabled, creating two inconsistent entry points. Update the shared toggle export in shared.tsx so it re-exports the canonical Toggle or forwards the full ToggleProps surface, and ensure ViewVisibilitySettings and similar settings components can rely on the shared import path for all supported props.src/components/agent/AgentChatPane.tsx (1)
63-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSolid consolidation of the duplicated save logic.
Reads fresh state via
getState()(safe for IPC callbacks) and correctly filters out in-flight streaming messages before persisting. One small nit: the.catch(console.error)doesn't includesessionIdin the log, making it harder to correlate failures across concurrent sessions in the console/log aggregator.🔍 Optional: add context to the error log
- window.electron?.piAgent.saveMessages(sessionId, saveable).catch(console.error); + window.electron?.piAgent.saveMessages(sessionId, saveable).catch((err) => + console.error(`[AgentChatPane] failed to persist transcript for session ${sessionId}:`, err) + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/agent/AgentChatPane.tsx` around lines 63 - 81, The save failure log in persistPiTranscript lacks session context, which makes concurrent transcript persistence errors hard to trace. Update the .catch(console.error) handling in persistPiTranscript so the failure log includes the sessionId alongside the error, using the same saveMessages call path and preserving the existing fire-and-forget behavior.src/lib/events.ts (1)
35-71: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGood consolidation; consider hardening the fixed-delay dispatch.
navigateAndRevealrelies on a fixedREVEAL_DELAY_MS(50ms) guess that the target view has mounted itsCustomEventlistener. Since this is now the single chokepoint for reveal navigation used across chat, kanban, project-overview, and ref chips (per the docstring), a slow mount (e.g. board/notes view with heavy data) will silently drop the event — the view switches but the intended item never gets revealed, with no error surfaced.A double
requestAnimationFramewait (mount happens before the second paint) is more robust than a magic-number timeout and is a drop-in replacement here.♻️ Proposed fix
-/** - * Delay (ms) between switching views and dispatching the reveal event. The - * target view needs one render cycle to mount its CustomEvent listener before - * we fire, otherwise the event is missed. Centralised here so the magic number - * lives in exactly one place. - */ -const REVEAL_DELAY_MS = 50; - type ActiveView = AppUIState["activeView"]; type SetView = (view: ActiveView) => void; /** * Switch to `view`, then—after the target view has had a render cycle to mount * its listener—dispatch `event`. Consolidates the - * `setView(...); setTimeout(() => window.dispatchEvent(...), 50)` idiom that + * `setView(...); setTimeout(() => window.dispatchEvent(...), 50)` idiom that * was duplicated across chat, kanban, project-overview, and ref chips. */ export function navigateAndReveal(setView: SetView, view: ActiveView, event: Event) { setView(view); - setTimeout(() => window.dispatchEvent(event), REVEAL_DELAY_MS); + // Wait for two paints so the target view's mount effect has definitely run, + // instead of guessing a fixed timeout. + requestAnimationFrame(() => requestAnimationFrame(() => window.dispatchEvent(event))); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/events.ts` around lines 35 - 71, The shared reveal helper still depends on a fixed timeout, which can miss the target view’s event listener if mounting is slower than expected. Update navigateAndReveal to wait for the next paint(s) instead of using REVEAL_DELAY_MS, keeping the same setView and window.dispatchEvent flow but replacing the magic-number delay with a more reliable readiness wait before firing CairnEvents in revealNote, revealCard, and revealColumn.src/components/graph/NodeTypeChip.tsx (1)
51-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
aria-pressedto the toggle chip for accessibility.The interactive chip acts as a pressed/unpressed toggle but doesn't expose that state to assistive tech.
♿ Proposed fix
<button onClick={onClick} + aria-pressed={active} className={cn(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/graph/NodeTypeChip.tsx` around lines 51 - 70, The toggle chip rendered in NodeTypeChip as the button element is acting like a pressed/unpressed control but does not expose its state to assistive technologies. Update the button in NodeTypeChip to include an aria-pressed attribute driven by the existing active state so screen readers can announce the toggle state correctly.src/components/notes/MermaidDiagram.tsx (1)
19-19: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMermaid diagrams won't repaint on live theme toggle.
getIsDark()is read once per render call insidegetMermaid, but neitherDiagramModal's norMermaidDiagram's render effect depends on theme, so an already-rendered diagram keeps its stale theme colors if the user switches themes while it's mounted — inconsistent with the newly-reactive CodeBlock/DiffViewer/GitView.♻️ Proposed fix
+import { useIsDark } from "`@/hooks/useIsDark`"; ... export function MermaidDiagram({ chart }: Props) { const containerRef = useRef<HTMLDivElement>(null); const [error, setError] = useState<string | null>(null); const [expanded, setExpanded] = useState(false); const id = useId().replace(/:/g, ""); + const isDark = useIsDark(); useEffect(() => { if (!containerRef.current) return; let cancelled = false; async function render() { const { mermaid } = await getMermaid(id); ... } render(); return () => { cancelled = true; }; - }, [chart, id]); + }, [chart, id, isDark]);Apply the same pattern to
DiagramModal.Also applies to: 27-61, 114-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/notes/MermaidDiagram.tsx` at line 19, Mermaid diagrams are not reacting to live theme changes, so the rendered diagram can keep stale colors after a theme toggle. Update the theme-dependent logic in MermaidDiagram and DiagramModal so they re-render when the active theme changes, using the same reactive pattern already used by CodeBlock, DiffViewer, and GitView; make sure the getMermaid flow and the modal render effect both depend on the theme state instead of reading getIsDark() only once.src/components/notes/note-pdf-export.ts (1)
21-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBroad selector risks matching unrelated content.
div.my-4matches any element with that single (common) Tailwind margin utility class, not justCodeBlock's wrapper, andblock.querySelector("div")for the header isn't scoped to a direct child. An unrelated note element that happens to sharemy-4and contain a<pre>would have its first<div>stripped or, in light mode, unrelated span colors rewritten.♻️ Suggested tightening
Add a stable marker (e.g.
data-codeblockonCodeBlock's rootdivand its headerdiv) and match on that instead of the generic utility class:- for (const block of root.querySelectorAll<HTMLElement>("div.my-4")) { - const pre = block.querySelector<HTMLElement>("pre"); - const header = block.querySelector<HTMLElement>("div"); + for (const block of root.querySelectorAll<HTMLElement>("div[data-codeblock]")) { + const pre = block.querySelector<HTMLElement>(":scope > pre"); + const header = block.querySelector<HTMLElement>(":scope > div[data-codeblock-header]");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/notes/note-pdf-export.ts` around lines 21 - 24, The PDF export logic in note-pdf-export is using overly broad selectors that can match unrelated note content. Update the loop in the export routine to target a stable CodeBlock marker instead of div.my-4, and scope the header lookup to the CodeBlock-specific header element rather than any nested div. Add matching data attributes on CodeBlock’s root and header elements so the selector in note-pdf-export only processes the intended code block structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture-review.md`:
- Around line 216-220: The `card-detail.tsx` / `card-detail-panel.tsx`
duplication recommendation is stale because the `CardDetailBody` refactor has
already shipped, so this section is now contradictory with the progress log.
Update the architecture review entry by removing the open-work guidance or
moving it into the completed work section, and keep the note aligned with the
actual refactor status in the document.
In `@src/components/agent/AgentMessageBubble.tsx`:
- Line 256: The message bubble styling in AgentMessageBubble is hardcoding white
text instead of using the accent foreground design token. Update the className
on the bubble container to use the CSS variable for the accent foreground (the
same token used elsewhere for themed contrast) rather than text-white, so the
text color follows per-theme accessibility rules.
In `@src/components/chat/chat-panel/MarkdownContent.tsx`:
- Around line 99-121: The MarkdownContent user-bubble styling is using raw
`text-white` and hardcoded `#ffffff`-based `color-mix` values, which breaks the
token-only color guideline. Update the user-path token definitions in
`MarkdownContent` (such as `strongColor`, `headingColor`, `listColor`,
`codeClass`, `quoteClass`, `ruleBorder`, `thClass`, and `tdClass`) to use CSS
variables like the existing accent/foreground tokens instead of literal white
values. Keep the same contrast behavior by expressing the alpha variants with
`color-mix(in srgb, var(--token) X%, transparent)` rather than hardcoded color
names.
In `@src/components/kanban/archive-view.tsx`:
- Around line 84-92: The archive card content currently nests
NoteMarkdownPreview inside the button in archive-view, which can create invalid
interactive markup when the preview renders links or other controls. Update the
card rendering to avoid placing markdown preview inside the clickable button:
either move the preview outside the button and make the card container handle
click/keyboard activation, or replace it with a non-interactive plain-text
snippet in this section. Use the onOpenCard handler and the card
button/container structure in ArchiveView to locate the change.
- Line 60: The alpha background utility classes in archive-view should follow
the project color rule instead of using the slash syntax. Update the affected
className values in the archive view markup so any hover or background alpha
tied to var(--danger) (and the other referenced spots in this component) uses
color-mix(in srgb, var(--token) X%, transparent) equivalents. Locate the
affected styles in the archive-view component and replace the var(--token)/10
usage consistently.
- Around line 120-130: In archive-view.tsx, the icon-only action buttons for
restore and permanent delete in the archive card controls need accessible names.
Update the button elements used with restoreCard and deleteCard to include
distinct aria-label values that describe each action, while keeping the existing
Tooltip and icon behavior intact so screen-reader users can identify the
controls.
- Around line 56-60: The delete actions in ArchiveView are immediately
performing permanent removal without confirmation. Update the destructive
handlers in the archive-view.tsx component, including the bulk delete button
that calls deleteCard over filtered cards and the individual delete path around
the other referenced deleteCard usage, to prompt the user for confirmation
before proceeding. Use the existing ArchiveView UI logic and deleteCard
invocation points to gate the actual delete behind a confirm step, so no
permanent delete happens on a single click.
In `@src/components/kanban/card-detail-body.tsx`:
- Around line 122-128: The description editor container in card-detail-body.tsx
is clickable but not keyboard-accessible, so update the div used in the
description area to behave like an interactive control. Add the appropriate role
and tabIndex, and handle Enter/Space in the same onClick handler path that
setsIsEditingDesc(true), while keeping the existing closest("a, button") guard
intact so links and buttons still do not trigger edit mode.
- Line 183: The note-open action in the CardDetailBody button only calls
revealNote and leaves the current surface open. Update the button handler in
card-detail-body.tsx so it also invokes onClose immediately after opening the
linked note, keeping the prop contract consistent. Use the existing
revealNote(setView, note.id) call as the entry point and add the close behavior
in the same onClick flow.
In `@src/components/notes/CodeBlock.tsx`:
- Around line 34-95: The `buildPalette` function in `CodeBlock` hardcodes the
`hljs-strong` colors instead of using the shared syntax tokens, which breaks the
PDF light-theme remap. Add a `strong` entry to `SYNTAX_COLORS` in
`syntax-palette.ts`, then update `buildPalette` to read `hljs-strong` from
`c("strong")` so it participates in `DARK_TO_LIGHT` like the other tokens. Keep
the `DARK` and `LIGHT` palettes derived only from the shared palette data.
In `@src/hooks/useIsDark.ts`:
- Around line 26-32: The server snapshot in useIsDark is hardcoded to dark,
which can mismatch the DOM during hydration when the theme has already been set
to light. Update useIsDark to reuse getIsDark for the third useSyncExternalStore
argument so the initial client render reads the current DOM state while still
falling back safely when document is unavailable.
---
Outside diff comments:
In `@src/components/settings/ViewVisibilitySettings.tsx`:
- Around line 42-55: The Toggle in ViewVisibilitySettings is using a hardcoded
accessible name, so both rows announce the same label. Update the Toggle inside
the ["Overview", "Notes"] map to include the per-row label variable in its
accessible label, matching the pattern already used by the toggleable rows
elsewhere in the component, so each switch is uniquely identified by its row
name.
---
Nitpick comments:
In `@src/components/agent/AgentChatPane.tsx`:
- Around line 63-81: The save failure log in persistPiTranscript lacks session
context, which makes concurrent transcript persistence errors hard to trace.
Update the .catch(console.error) handling in persistPiTranscript so the failure
log includes the sessionId alongside the error, using the same saveMessages call
path and preserving the existing fire-and-forget behavior.
In `@src/components/graph/NodeTypeChip.tsx`:
- Around line 51-70: The toggle chip rendered in NodeTypeChip as the button
element is acting like a pressed/unpressed control but does not expose its state
to assistive technologies. Update the button in NodeTypeChip to include an
aria-pressed attribute driven by the existing active state so screen readers can
announce the toggle state correctly.
In `@src/components/notes/MermaidDiagram.tsx`:
- Line 19: Mermaid diagrams are not reacting to live theme changes, so the
rendered diagram can keep stale colors after a theme toggle. Update the
theme-dependent logic in MermaidDiagram and DiagramModal so they re-render when
the active theme changes, using the same reactive pattern already used by
CodeBlock, DiffViewer, and GitView; make sure the getMermaid flow and the modal
render effect both depend on the theme state instead of reading getIsDark() only
once.
In `@src/components/notes/note-pdf-export.ts`:
- Around line 21-24: The PDF export logic in note-pdf-export is using overly
broad selectors that can match unrelated note content. Update the loop in the
export routine to target a stable CodeBlock marker instead of div.my-4, and
scope the header lookup to the CodeBlock-specific header element rather than any
nested div. Add matching data attributes on CodeBlock’s root and header elements
so the selector in note-pdf-export only processes the intended code block
structure.
In `@src/components/settings/shared.tsx`:
- Line 6: The shared Toggle wrapper currently exposes only a narrow prop subset,
while other settings code imports the UI Toggle directly for additional props
like disabled, creating two inconsistent entry points. Update the shared toggle
export in shared.tsx so it re-exports the canonical Toggle or forwards the full
ToggleProps surface, and ensure ViewVisibilitySettings and similar settings
components can rely on the shared import path for all supported props.
In `@src/lib/events.ts`:
- Around line 35-71: The shared reveal helper still depends on a fixed timeout,
which can miss the target view’s event listener if mounting is slower than
expected. Update navigateAndReveal to wait for the next paint(s) instead of
using REVEAL_DELAY_MS, keeping the same setView and window.dispatchEvent flow
but replacing the magic-number delay with a more reliable readiness wait before
firing CairnEvents in revealNote, revealCard, and revealColumn.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 61b809a3-d1d6-4796-9c3d-74f87ec58913
📒 Files selected for processing (58)
changelogs/v2.3.6.mddocs/architecture-review.mdsrc/components/agent/AgentChatPane.tsxsrc/components/agent/AgentEditor.tsxsrc/components/agent/AgentEmptyState.tsxsrc/components/agent/AgentMessageBubble.tsxsrc/components/agent/AgentSessionTab.tsxsrc/components/agent/DiffViewer.tsxsrc/components/agent/GitView.tsxsrc/components/agent/sessionUtils.tssrc/components/chat/PreviewPane.tsxsrc/components/chat/chat-panel/ChatMessageBubble.tsxsrc/components/chat/chat-panel/MarkdownContent.tsxsrc/components/chat/chat-panel/QuestionForm.tsxsrc/components/chat/chat-panel/ThinkingPanel.tsxsrc/components/chat/chat-panel/ToolCallIndicator.tsxsrc/components/flow/nodes/NoteRefNode.tsxsrc/components/flow/nodes/TaskRefNode.tsxsrc/components/graph/ForceGraphCanvas.tsxsrc/components/graph/KnowledgeGraphView.tsxsrc/components/graph/NodeTypeChip.tsxsrc/components/graph/RadialTreeCanvas.tsxsrc/components/graph/TableCanvas.tsxsrc/components/graph/analyticsHooks.tssrc/components/graph/analyticsUtils.tssrc/components/insights/InsightsView.tsxsrc/components/kanban/archive-view.tsxsrc/components/kanban/board.tsxsrc/components/kanban/card-detail-body.tsxsrc/components/kanban/card-detail-panel.tsxsrc/components/kanban/card-detail.tsxsrc/components/layout/project-overview/index.tsxsrc/components/layout/project-overview/sections.tsxsrc/components/layout/project-overview/useProjectMetrics.tssrc/components/notes/CodeBlock.tsxsrc/components/notes/DashboardApiModal.tsxsrc/components/notes/MermaidDiagram.tsxsrc/components/notes/NoteMarkdownPreview.tsxsrc/components/notes/dashboard-view.tsxsrc/components/notes/markdown-code-fence.tsxsrc/components/notes/note-editor.tsxsrc/components/notes/note-pdf-export.tssrc/components/onboarding/StepMCP.tsxsrc/components/search/search-panel.tsxsrc/components/settings/AgentSettings.tsxsrc/components/settings/MCPSettings.tsxsrc/components/settings/ViewVisibilitySettings.tsxsrc/components/settings/shared.tsxsrc/components/shared/cairn-ref-chip.tsxsrc/components/ui/toggle.tsxsrc/hooks/useCopyToClipboard.tssrc/hooks/useIsDark.tssrc/lib/constants.tssrc/lib/editor-theme.tssrc/lib/events.tssrc/lib/syntax-palette.tssrc/lib/utils.tssrc/store/index.ts
💤 Files with no reviewable changes (1)
- src/components/agent/sessionUtils.ts
Replace hardcoded `text-white` and `#ffffff` color-mix values with the theme-aware `--accent-fg` token across user message bubbles. Plain white fails AA contrast on the dark accent surface; `--accent-fg` is theme-aware and meets AA on the accent background. Changes: - AgentMessageBubble and ChatMessageBubble: swap `text-white` for `text-[var(--accent-fg)]` on the accent bubble. - MarkdownContent: migrate strong/heading/list colors, code, quote, rule, table header/cell, and link classes to `--accent-fg`-based tokens (including color-mix alpha variants). Update the accompanying comment to document the contrast rationale. - AgentChatPane: add contextual logging to the transcript persistence error handler for easier debugging. - Update architecture-review doc to mark the card-detail dedupe (4.1) as done in Wave 2.
9 of 16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR is a major maintenance release focused on architectural cleanup, code deduplication, and UX polish. Key improvements include consolidating shared UI primitives (hooks and components), unifying theme synchronization across visual components, and modularizing oversized files to improve maintainability.
useIsDark,useCopyToClipboard,NodeTypeChip, and a unifiedToggleswitch) to replace ad-hoc logic across the app.--info) across all surfaces (flow nodes, modals, cards) for a consistent design language.Type of change
Checklist
npm run type-check:allpassesnpm run lintpassesnpm testpassesnpm run test:e2epassestext-[Npx]pixel font classes — rem equivalents onlyhandle()and returnIpcResult<T>schema.tselectron/db/queries.tsdependenciesordevDependenciesadded toROLE_MAPinscripts/generate-licenses.jsandlicenses.jsonregeneratedelectron/mcp/tools/index.tsdispatch +electron/lib/tool-schemas.tsZod schemacompilescript andelectron/bundle-guard.test.ts(if applicable)Notes for reviewer
The architecture review documentation (
docs/architecture-review.md) has been updated to reflect the completion of Waves 1, 2, and 3. Most changes are internal structural cleanups with no intended change to visual output, except for the explicit theme-sync fix and color-alignment.Summary by CodeRabbit
New Features
Bug Fixes