Repository navigation
Community personalities + Writing Style + leaner agent prompt - #123
Merged
Merged
Conversation
LLM-generated diagrams often give subgraphs (clusters) an explicit light pastel fill while mermaid's label colour follows the theme — light text in dark mode becomes invisible on a light fill. After render, read each cluster's actual fill luminance and pin the label to a contrasting dark/light colour, in both the inline and fullscreen views.
A personality is a set of behavioral rules appended to the chat system prompt as a delimited style layer (never a replacement identity). Pick one next to the model selector in the chat input; Default adds nothing. - personalities.json manifest fetch/cache/IPC (mirrors providers) - Zod schema rejects 'You are …' openings + thin one-liner prompts - withPersonality() injection in the main-process chat loop (covers the default prompt, the graph-view override, and token-usage recompute) - aiConfig.installedPersonalities + personalityId (persisted via localStorage + config-cache whitelist), actions for install/remove/ create/select - PersonalityPicker in ChatInputArea, BrowsePersonalitiesModal, and a Chat personality section in Settings → AI & Chat - changelog v2.6.12 + What's New entry; tests for schema, fetch, store, config-cache, and injection
…m prompt The pi-agent prompt listed every tool under '## Coding tools' and '## Cairn tools' (and '## Available tools' in plan mode), but the model already receives each tool's full definition in the tools array — the listing was pure overhead. Removed both sections; the base identity, context, mandatory workflow, and coding guidelines are untouched. - ~318 tokens saved per turn on the coding agent (~27% of the prompt) - new pi-agent-prompt-trim.test.ts: offline assertions that the sections are gone + deterministic token savings; opt-in live A/B (CAIRN_LIVE_TESTS=1) that replays a tool-selection task set through runToolLoop with the legacy vs trimmed prompt and asserts the trimmed prompt selects the correct tools at least as often with fewer prompt tokens (verified live: 3/7 vs 2/7 correct, 4% fewer tokens)
…cess Two related bugs when no personality is installed yet: - useShallow selectors returned a fresh array from `?? []` inside the selector, breaking snapshot caching (React infinite-loop guard fired as 'getSnapshot should be cached' / 'Maximum update depth exceeded'). Hoist the fallback out of the selector (same pattern as ProviderManager). - PersonalityPicker read `installedPersonalities.find` on the possibly- undefined store field, throwing 'Cannot read properties of undefined'. All list access now goes through the null-safe personalityList.
…t, get_user_writing_style tool A Writing Style system so AI-drafted content (emails, replies, notes, PRDs) sounds like the user. Guided 5-step wizard generates a full style guide plus a condensed cheat sheet; chat, the coding agent, and the MCP server can all read them via get_user_writing_style (lazy — fetched only when drafting). - DB: migration 41 user_style single-row table + queries (upsert/clear, persona JSON parsed defensively); re-exported from db/queries - Tool: TOOL_SCHEMAS + labels + chat-executor case + MCP executor case; added to CAIRN_TOOL_NAMES + PLAN_MODE_ALLOWED for the coding agent; system-prompt hints in chat (buildSystemPrompt) and agent guidelines. Returns configured:false when unset — never invents a style. - Wizard (Settings → Writing Style): persona → paste samples and/or analyse recent notes & tasks → gap questions → generate full guide → generate cheat sheet, both editable before saving (source guided/analyzed) - Generation: user-style:generate IPC → callLLM with structured prompts (lib/user-style-prompt.ts); new 'writing-style' usage source - Settings: Writing Style nav item + section with previews/regenerate/clear - changelog v2.6.12 + What's New entry; tests: queries, executor (chat), MCP baseline, store slice
setPersonality(null) cleared personalityId in the store, but the backend config cache kept the previous value (Electron structured-clone drops undefined, so the cache's 'preserve when not a string' rule never saw the clear). On the next launch the cache won the hydration merge and re-selected the last personality — so 'None' could never be kept as default. - 'None' is now stored as an explicit null (survives JSON + IPC), and config-cache treats an incoming null as CLEAR (absent = preserve, for connection-only saves) - picker: trigger + row now read 'None — no personality' instead of the ambiguous 'Personality'/'Default' - AISettings: explicit 'None' row with Active/Inactive states (replaces the misleading per-row 'Default' label) - tests: store null-persistence + config-cache null-clear
|
Warning Review limit reached
Next review available in: 8 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (23)
📝 WalkthroughWalkthroughThe change adds chat personalities with community browsing, installation, custom creation, and prompt application. It adds guided writing-style generation, persistence, editing, and retrieval tools. It also trims agent prompts and improves Mermaid subgraph label contrast. ChangesChat Personalities
Writing Style
Agent Prompt Optimization
Mermaid Label Contrast
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PersonalityPicker
participant ChatPanel
participant ElectronChat
participant LLM
User->>PersonalityPicker: Select personality
PersonalityPicker->>ChatPanel: Update active personality
ChatPanel->>ElectronChat: Send personality name and prompt
ElectronChat->>LLM: Apply personality to system prompt
LLM-->>User: Return chat response
sequenceDiagram
participant User
participant UserStyleWizardModal
participant UserStyleIPC
participant LLM
participant ChatTool
User->>UserStyleWizardModal: Submit persona and writing samples
UserStyleWizardModal->>UserStyleIPC: Generate full guide
UserStyleIPC->>LLM: Send style-generation prompt
LLM-->>UserStyleIPC: Return Markdown guide
UserStyleWizardModal->>UserStyleIPC: Generate condensed guide
UserStyleIPC->>LLM: Send condensation prompt
LLM-->>UserStyleIPC: Return Markdown cheatsheet
ChatTool->>UserStyleIPC: Request stored writing style
UserStyleIPC-->>ChatTool: Return cheatsheet or full guide
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 14
🧹 Nitpick comments (2)
electron/ipc/chat.ts (1)
163-163: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHoist the composed system prompt into one constant.
Line 239 rebuilds the prompt on every usage round.
buildSystemPromptembedsnew Date().toLocaleDateString(...), so the recomputed string can differ from the string sent at line 163 if a turn crosses midnight.calculatePromptBreakdownthen skips the in-array system message and measures a different text than the one sent. Compute the prompt once and reuse it.♻️ Proposed refactor
+ const systemContent = withPersonality(buildSystemPrompt(req), req.personality); + const messages: OpenAIMessage[] = [ { role: resolveSystemRole({ isReasoningModel: req.config?.isReasoningModel, baseUrl, provider, modelId: model }), - content: withPersonality(buildSystemPrompt(req), req.personality), + content: systemContent, },try { - const rawBreakdown = calculatePromptBreakdown(withPersonality(buildSystemPrompt(req), req.personality), messages, allTools); + const rawBreakdown = calculatePromptBreakdown(systemContent, messages, allTools);Also applies to: 239-239
🤖 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 `@electron/ipc/chat.ts` at line 163, In the request handling flow around the system message construction, compute the composed prompt from buildSystemPrompt(req) and withPersonality once in a shared constant, then reuse that constant both for the message content and the later calculatePromptBreakdown usage. Remove the second prompt reconstruction so both paths measure and send the identical prompt.src/components/notes/MermaidDiagram.test.ts (1)
4-35: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftAdd DOM-level coverage for the contrast correction.
These tests cover only
parseColorandcolorLuminance. They do not exerciseenforceClusterLabelContrast, the inline render path at Line 177, or the modal render path at Line 123.Add a DOM test with SVG and HTML cluster labels. Assert the applied CSS properties for dark, light, and medium fills. This will catch selector, cascade, and contrast regressions.
🤖 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.test.ts` around lines 4 - 35, Add DOM-level tests covering enforceClusterLabelContrast through both the inline render path and modal render path, using SVG and HTML cluster labels with dark, light, and medium fills. Assert the resulting CSS properties on each label, including the expected contrast correction, so selector and cascade behavior are exercised rather than testing only parseColor and colorLuminance.
🤖 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 `@electron/ipc/chat-executor.ts`:
- Around line 290-310: The writing-style handlers return only the requested
guide even when it is empty, despite the record being configured with the
alternate guide. Update the get_user_writing_style logic in
electron/ipc/chat-executor.ts (lines 290-310) and the corresponding MCP handler
in electron/mcp/tools/index.ts (lines 182-203) to fall back to fullGuide when
cheatsheet is empty and to cheatsheet when fullGuide is empty. Add coverage in
electron/ipc/chat-executor.test.ts (lines 1371-1388) for full-guide-only and
cheatsheet-only records, preserving the configured response metadata.
In `@electron/lib/pi-agent-prompt-trim.test.ts`:
- Around line 179-188: Update the onUsage callback passed to runToolLoop so
promptTokens accumulates usage across all tool-loop rounds by adding each pt
value instead of overwriting the previous total. Preserve the existing
toolErrors and called tracking behavior.
- Around line 212-213: Update electron/db/client.ts so Electron-specific
BetterSqlite3 binding resolution happens inside initDb rather than at module
load, and export a test-only createInMemoryDb() factory. Replace both direct new
BetterSqlite3(":memory:") constructions in the test with createInMemoryDb(),
using the factory for both database handles.
In `@electron/lib/pi-agent-prompt.ts`:
- Line 98: Update the Plan Mode prompt text around the ensure_note PRD
instructions to include the same get_user_writing_style requirement used by the
execute prompt, before drafting or updating the PRD. Preserve the existing plan
title and note-reuse guidance while ensuring the configured writing style is
applied to all Plan Mode PRD content.
In `@electron/lib/tools.ts`:
- Around line 191-197: Define one shared maximum-length constant for personality
prompts and enforce it both in the registry schema/custom-personality form
before persistence and in withPersonality before composing the system prompt.
Ensure overly long prompts are rejected or truncated consistently, and reuse the
same constant across all validation and runtime paths.
In `@src/components/notes/MermaidDiagram.tsx`:
- Line 102: Update the label color assignment in MermaidDiagram to use the
existing semantic CSS custom properties for the dark and light label colors
instead of hardcoded hex values, while preserving the luminance-based selection.
- Around line 100-102: Update the label-color selection in the Mermaid diagram
rendering logic around colorLuminance to compute WCAG contrast ratios between
the fill and both candidate colors, then choose whichever candidate has higher
contrast instead of using the fixed 0.5 luminance threshold. Add a medium-gray
test covering rgb(128, 128, 128), and adjust the candidate palette if the
implementation must guarantee a 4.5:1 contrast ratio.
- Around line 93-102: Update enforceClusterLabelContrast to handle both Mermaid
label variants: continue applying the computed contrast color via the SVG fill
attribute for .cluster-label text elements, and apply it via CSS color for
.cluster-label span HTML labels. Define the light and dark label color constants
before processing labels, and ensure both label types use the same
luminance-based selection.
- Around line 98-101: Update the rectangle color handling around attrFill, fill,
and colorLuminance so that when the attribute color fails to parse, retry
colorLuminance with getComputedStyle(rect).fill before continuing. Preserve the
existing skip behavior only when both the attribute and computed-style colors
are invalid, while retaining the current handling for missing or "none"
attributes.
In `@src/components/settings/AISettings.tsx`:
- Around line 307-358: Update the personality selectors in AISettings, including
the “None” row and each installed row rendered by the installedPersonalities
map, to use keyboard-operable button elements instead of clickable divs.
Preserve their selection behavior and styling, and restructure each installed
row so the remove control remains a sibling rather than a nested interactive
element; retain removePersonality and event propagation behavior.
In `@src/components/settings/UserStyleSettings.tsx`:
- Around line 46-65: Update regenerateCheatsheet to catch failures from
generateUserStyle and saveUserStyle, render an appropriate failure state, and
prevent the rejection from escaping the void click handler. Explicitly detect
when window.electron?.generateUserStyle is unavailable and report that as a
failure; retain setRegenerating(false) in the existing cleanup path.
In `@src/components/settings/UserStyleWizardModal.tsx`:
- Around line 305-328: Update the footer actions in UserStyleWizardModal to
render a Next button for steps 0 and 1, advancing via setStep when clicked.
Disable it while busy or when canProceed(step) is false, while preserving the
existing Cancel/Back behavior and generation actions for later steps.
In `@src/components/ui/personality-picker.tsx`:
- Around line 200-207: Update the remove button in the personality picker to
remain visually discoverable when keyboard-focused, while preserving the hover
visibility behavior; add a reliable accessible name to the button in addition to
the existing title, using the surrounding removePersonality control.
In `@src/store/slices/ui.ts`:
- Around line 789-819: Restrict name-based deduplication in
installCommunityPersonality to existing rows whose source is "community", both
in the initial existing lookup and the matchIdx lookup. Continue matching any
row by communityId, but prevent custom personalities with the same name from
being replaced.
---
Nitpick comments:
In `@electron/ipc/chat.ts`:
- Line 163: In the request handling flow around the system message construction,
compute the composed prompt from buildSystemPrompt(req) and withPersonality once
in a shared constant, then reuse that constant both for the message content and
the later calculatePromptBreakdown usage. Remove the second prompt
reconstruction so both paths measure and send the identical prompt.
In `@src/components/notes/MermaidDiagram.test.ts`:
- Around line 4-35: Add DOM-level tests covering enforceClusterLabelContrast
through both the inline render path and modal render path, using SVG and HTML
cluster labels with dark, light, and medium fills. Assert the resulting CSS
properties on each label, including the expected contrast correction, so
selector and cascade behavior are exercised rather than testing only parseColor
and colorLuminance.
🪄 Autofix
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: 619e5b30-3d38-4cf3-b68f-2965ed5656a4
📒 Files selected for processing (44)
changelogs/v2.6.12.mdelectron/db/queries.tselectron/db/schema.tselectron/db/usage-queries.tselectron/db/user-style-queries.test.tselectron/db/user-style-queries.tselectron/ipc/chat-executor.test.tselectron/ipc/chat-executor.tselectron/ipc/chat.tselectron/ipc/community-registry-handlers.tselectron/ipc/handlers.tselectron/ipc/user-style-handlers.tselectron/lib/community-registry.test.tselectron/lib/community-registry.tselectron/lib/config-cache.test.tselectron/lib/config-cache.tselectron/lib/pi-agent-loop.tselectron/lib/pi-agent-prompt-trim.test.tselectron/lib/pi-agent-prompt.tselectron/lib/tool-schemas.tselectron/lib/tools.tselectron/lib/user-style-prompt.tselectron/lib/with-personality.test.tselectron/mcp/tools/index.tselectron/preload.tsscripts/features.config.jsshared/chat/registry-schema.tssrc/components/chat/BrowsePersonalitiesModal.tsxsrc/components/chat/ChatInputArea.tsxsrc/components/chat/chat-panel/index.tsxsrc/components/notes/MermaidDiagram.test.tssrc/components/notes/MermaidDiagram.tsxsrc/components/settings/AISettings.tsxsrc/components/settings/UserStyleSettings.tsxsrc/components/settings/UserStyleWizardModal.tsxsrc/components/settings/settings-view.tsxsrc/components/ui/personality-picker.tsxsrc/hooks/useChatStream.tssrc/store/index.tssrc/store/slices/ui.test.tssrc/store/slices/ui.tssrc/store/slices/user-style.test.tssrc/store/slices/user-style.tssrc/types/index.ts
…elect dropdown - Steps 0-1 had no way to advance: only the Generate buttons existed in the footer. Added Next (step 0, gated on persona; step 1 always), and 'Skip for now' on the generation steps so the Edit flow can move on without regenerating a prefilled guide. - Replaced the native <select> context picker with the app's styled Select component (shared dropdown styling + check gutter).
…utput The guided generator could return 'token soup' — long scrambled fragments spliced from the sample text (seen when a pasted doc / weak endpoint overwhelms the model). No decode bug: callLLM's SSE parsing is the same path the chat loop uses. Hardened generation: - Bigger token budget (maxTokens 8192) and lower temperature (0.3) for the generation call (callLLM now accepts opts.temperature/maxTokens) - Bounded raw material: pasted samples truncated to 2k chars each / 12k total so a giant document can't drown the prompt - Coherence gate: output must contain >=6 (full) / >=3 (cheatsheet) ## headings; otherwise retry once at temp 0.1, and if still unusable throw a clear error naming the model instead of showing/saving garbage - tests: guard thresholds + sample capping
…ed generate fn The wizard hit 'unusable output' — the model (deepseek-v4-flash) is non-deterministic and sometimes returns token soup. A live reproduction confirmed it also produces a clean 12-section guide, so the failure was a bad draw caught by the coherence gate, not a broken pipeline. - extract generateUserStyleMarkdown() (prompt build + callLLM + retry + gate) so the IPC handler and the test drive identical code - new gated live test (CAIRN_LIVE_TESTS=1): full guide AND cheat sheet must pass isUsableGuide against the configured endpoint — verified green against the ocgo proxy + deepseek-v4-flash
…ne-shots Root cause of the Writing Style 'unusable output': on reasoning-model gateways, LONG one-shot prose streamed back as token soup (verified: same request clean non-streamed, garbled streamed — and adding tools/max_tokens did not change it). Chat/agent loops were unaffected because their turns are short and tool-oriented. - callLLM gains stream:false support (reads the final message + usage) and the Writing Style generator uses it — proven clean end-to-end on deepseek-v4-flash via opencode.ai/zen/go (full guide 12 headings) - callLLM's streaming path now consumes via consumeAssistantStream (the shared buffered SSE parser the chat/agent loops use) instead of the old inline split-per-chunk loop that could mangle SSE events split across TCP reads and ignored reasoning fields — fixing all one-shot tools - dynamic import to avoid the llm <-> llm-stream module cycle - tests: llm-stream/llm-sendable/llm-url/token-breakdown + user-style guards all green
…arkdown preview Wizard now generates in real time instead of a one-shot spinner: - user-style:generateStream runs runToolLoop with a READ-ONLY tool set (get_project_context_pack, search_notes, get_note, search_tasks, get_task) when 'analyse my notes' is on — the guide streams into the preview and each note-read shows as a live tool chip; no write tools, no get_active_context (IDs passed in the prompt). Verified live: toolCalls=[...search_notes, get_note...] → 12-heading guide. - 'Optimize' button on the full-guide step restructures an existing guide into the canonical 12-section format (shared buildUserStylePromptPair so one-shot + streaming can't drift; optimize gated like full). - Edit ↔ Preview toggle on the guide/cheat-sheet steps using the shared NoteMarkdownPreview (inline); Settings previews render markdown too. - live test: streaming/tools path + one-shot full/cheatsheet all green on deepseek-v4-flash via zen/go.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/components/settings/UserStyleWizardModal.tsx (1)
461-465: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply
canProceed(1)to the step 1 Next button.
canProceed(1)requires at least one sample, but the button ignores it. A user can advance with no samples and then generate a style guide from persona text only. If that path is intentional, delete the unused branch incanProceedto avoid a misleading contract.🐛 Proposed fix
{step === 1 && ( - <Button size="sm" disabled={busy} onClick={() => setStep(2)}> + <Button size="sm" disabled={busy || !canProceed(1)} onClick={() => setStep(2)}> Next </Button> )}🤖 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/UserStyleWizardModal.tsx` around lines 461 - 465, Update the step 1 Next Button in UserStyleWizardModal to disable when either busy or canProceed(1) is false, preventing advancement without a sample; preserve the existing click behavior.
🧹 Nitpick comments (2)
electron/ipc/user-style-handlers.ts (1)
115-119: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueValidate the precondition before you build the prompt.
Line 115 builds the prompt with
input.fullGuide ?? ""for thecheatsheetandoptimizesteps. Line 117 then rejects a missingfullGuide. Move the check above the builder so the code never constructs a prompt from an empty source. The same ordering exists in the streaming path at lines 195 and 200.♻️ Proposed reorder
- const { systemPrompt, userPrompt } = buildUserStylePromptPair(step, input); - if ((step === "cheatsheet" || step === "optimize") && !input.fullGuide) { throw new Error(step === "cheatsheet" ? "No full guide to condense." : "No full guide to optimize."); } + const { systemPrompt, userPrompt } = buildUserStylePromptPair(step, input);🤖 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 `@electron/ipc/user-style-handlers.ts` around lines 115 - 119, Move the missing-fullGuide validation above buildUserStylePromptPair in the non-streaming handler, so cheatsheet and optimize steps reject before prompt construction. Apply the same reorder in the streaming path: validate the precondition before its buildUserStylePromptPair call while preserving the existing error messages and other steps’ behavior.electron/lib/user-style-generation.live.test.ts (1)
36-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport the shared tool set and prompt builder instead of copying them.
Lines 36-38 duplicate
WRITING_STYLE_TOOLS, and lines 105-107 duplicate thefullsystem prompt and the "Active context" hint. The handler already exportsbuildUserStylePromptPair. If the handler changes these strings, the live test still passes against stale text. ExportWRITING_STYLE_TOOLSandwritingStyleToolsOverridefromelectron/ipc/user-style-handlers.tsand reuse them here.Also applies to: 105-113
🤖 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 `@electron/lib/user-style-generation.live.test.ts` around lines 36 - 38, Replace the duplicated WRITING_STYLE_TOOLS set and full prompt/“Active context” text in the live test with imports from the user-style handler. Export and reuse WRITING_STYLE_TOOLS, writingStyleToolsOverride, and the existing buildUserStylePromptPair from user-style-handlers.ts so the test always reflects the handler’s current configuration.
🤖 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 `@electron/ipc/user-style-handlers.ts`:
- Around line 178-180: Update the stream request cleanup in the generation
handler’s finally block to delete the abortControllers entry only when it still
references that run’s AbortController, preserving a newer controller created for
the same event.sender.id. Apply the same ownership check in the user-style:abort
handler if it removes the map entry, so abort operations cannot clear a newer
generation’s controller.
In `@src/components/settings/UserStyleWizardModal.tsx`:
- Around line 452-454: Update the footer controls in UserStyleWizardModal so a
running streaming generation exposes a Cancel action instead of disabling the
only control. When the Cancel action is invoked, call
window.electron?.abortUserStyleStream?.() and reset busy, streaming, and the
listener array, while preserving the existing Cancel/Back behavior when
streaming is false.
- Around line 189-198: Stop passing credentials from the renderer in the
user-style generation flow: remove the config payload, including
aiConfig.apiKey, from the electron.generateUserStyleStream call near the
activeProject lookup, and remove the corresponding optional config field from
the user-style:generateStream request type. Keep credential resolution in the
main process via resolveChatConfig() and resolveLlmApiKey().
---
Duplicate comments:
In `@src/components/settings/UserStyleWizardModal.tsx`:
- Around line 461-465: Update the step 1 Next Button in UserStyleWizardModal to
disable when either busy or canProceed(1) is false, preventing advancement
without a sample; preserve the existing click behavior.
---
Nitpick comments:
In `@electron/ipc/user-style-handlers.ts`:
- Around line 115-119: Move the missing-fullGuide validation above
buildUserStylePromptPair in the non-streaming handler, so cheatsheet and
optimize steps reject before prompt construction. Apply the same reorder in the
streaming path: validate the precondition before its buildUserStylePromptPair
call while preserving the existing error messages and other steps’ behavior.
In `@electron/lib/user-style-generation.live.test.ts`:
- Around line 36-38: Replace the duplicated WRITING_STYLE_TOOLS set and full
prompt/“Active context” text in the live test with imports from the user-style
handler. Export and reuse WRITING_STYLE_TOOLS, writingStyleToolsOverride, and
the existing buildUserStylePromptPair from user-style-handlers.ts so the test
always reflects the handler’s current configuration.
🪄 Autofix
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: 3c4103bf-8a2e-4dfe-9cc4-65b9ecb0dfdd
📒 Files selected for processing (8)
electron/ipc/user-style-handlers.test.tselectron/ipc/user-style-handlers.tselectron/lib/llm.tselectron/lib/user-style-generation.live.test.tselectron/lib/user-style-prompt.tselectron/preload.tssrc/components/settings/UserStyleSettings.tsxsrc/components/settings/UserStyleWizardModal.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- electron/preload.ts
- src/components/settings/UserStyleSettings.tsx
- electron/lib/user-style-prompt.ts
…2x faster) Measured on deepseek-v4-flash via zen/go: 'none' cuts wall time roughly in half (full guide 38.7s→20.9s, 0 reasoning tokens) while still producing a usable 12-heading guide; 'low' ≈ default here. - callLLM: new opts.reasoningEffort → reasoning_effort in the body - buildChatCompletionsBody: optional reasoningEffort, threaded from ChatRequest.config through runToolLoop - writing-style one-shot + streaming generation both set 'none' (opt-in only — chat/agent keep the endpoint default) - live suite green: full 18.5s, cheat 33.7s, streamed+tools 42.6s
…/422) reasoning_effort is ignored by non-reasoning models but a strict OpenAI-compatible server may reject the unknown field. Both call paths now retry once without it on 400/422: - callLLM: rebuilds the body without reasoning_effort and re-posts - runToolLoop: doFetch + buildBody helpers, same retry for the streaming path - test: buildChatCompletionsBody includes/omits reasoning_effort correctly So the ~2x speedup from reasoning_effort:none stays for reasoning models, and non-reasoning models / strict endpoints fall back cleanly.
Functional: - get_user_writing_style falls back to the alternate guide when the requested one is empty (chat-executor + MCP + tests) - chat.ts: compose the system prompt ONCE so the token-usage breakdown measures the exact text sent (avoids date drift across a midnight turn) - plan-mode prompt now calls get_user_writing_style before drafting PRDs - user-style handlers: validate missing fullGuide BEFORE building the prompt; abort-controller map entry is only cleared when still owned by the run (prevents clearing a newer generation's controller) - pi-agent A/B test accumulates promptTokens across tool-loop rounds Security: - wizard no longer sends apiKey/baseUrl/model over IPC — main resolves via resolveChatConfig/resolveLlmApiKey Accessibility / UX: - AISettings personality rows are keyboard-operable buttons; remove is a sibling (no nested interactive) - wizard: Cancel action during streaming (aborts + resets); step-1 Next gated on canProceed(1) - personality-picker remove button focus-visible + aria-label Data integrity: - installCommunityPersonality name-dedup only matches community rows, so a custom personality with the same name is never replaced Robustness: - regenerateCheatsheet catches failures + reports when generation is unavailable - shared MAX_PERSONALITY_PROMPT_CHARS enforced in the registry schema (zod .max), the create forms (maxLength) and withPersonality (truncate) - Mermaid cluster labels use semantic --mermaid-label-* tokens, WCAG contrast-ratio selection, HTML .cluster-label span support, and a computed-style luminance fallback - live test reuses the handler's writingStyleToolsOverride + buildUserStylePromptPair (no duplicated config) Skipped (documented pattern / low value): createInMemoryDb() factory (AGENTS.md exempts test code from the ABI constraint — tests construct in-memory DBs directly) and Mermaid DOM-level tests (implementation now covered; existing unit tests remain).
…+ reasoning_effort none
Audit of one-shot LLM callers showed compaction was the last surface still
using the naive decoder.decode(value).split('\n')-per-chunk SSE loop — the
same bug fixed in callLLM (mangled records straddling TCP reads, ignored
reasoning fields). Rewrote it to consume via consumeAssistantStream, added
reasoning_effort:none (direct transform, ~2x faster) with a 400/422
fallback that retries without it for non-reasoning models.
Status across one-shot tools:
- callLLM (robust parser + options): commit message, PR description,
explain, PRD, Idea Flow summary, spawn-tasks, writing style
- compaction: NOW fixed (was the last buggy inline parser)
- tool-builder: already non-streaming (res.json) — no SSE bug; left as-is
because it's a tool-call path where reasoning_effort:none could degrade
the generated tool definition
- chat-subagent-loop: uses runToolLoop (robust)
Owner
Author
|
@coderabbitai are we good to go? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ddutchie/cairn"
pr="123"
echo "== Pull request =="
gh pr view "$pr" --repo "$repo" --json state,isDraft,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{state, isDraft, mergeStateStatus, reviewDecision, checks: [.statusCheckRollup[]? | {name, status, conclusion, detailsUrl}]}'
echo
echo "== Reviews =="
gh api "repos/$repo/pulls/$pr/reviews" --paginate \
--jq '.[] | {user: .user.login, state, submitted_at, body}'
echo
echo "== Open review threads =="
pr_id="$(gh api graphql -F owner="ddutchie" -F name="cairn" -F number="$pr" -f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
comments(first: 10) {
nodes { author { login } body path line originalLine }
}
}
}
}
}
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | .comments.nodes[] | {author: .author.login, path, line, originalLine, body}')"
if [ -n "$pr_id" ]; then
printf '%s\n' "$pr_id"
else
echo "No open review threads."
fiLength of output: 33067
All CI checks pass, including E2E tests. The PR is mergeable and not a draft. One unresolved CodeRabbit finding remains in Add and enforce a shared prompt-length limit, or resolve the finding with a valid rationale. After that, this PR is ready to go. You are interacting with an AI system. |
This was referenced Aug 11, 2026
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?
Three related additions plus fixes, on top of the branch's pre-existing mermaid fix:
get_user_writing_style({ mode: "cheatsheet" | "full" })to draft content in the user's voice — lazy, never injected into the prompt.pi-agent-prompt-trim.test.ts).Plus: mermaid subgraph label contrast fix, personality-picker selector stability fix, "None" persistence fix.
Type of change
Screenshots / recording
No screenshots attached — happy to add on request. The UI surface: a personality chip in the chat input footer, a Personality section in Settings → AI & Chat, and a new Settings → Writing Style section with a wizard modal.
Checklist
npm run type-check:allpassesnpm run lintpassesnpm testpasses (150 files / 2077 tests — live suites gated off by default)npm run test:e2epasses (not run locally; recommend running before merge — touches chat input + settings UI)scripts/features.config.js(v2.6.12-chat-personalities,v2.6.12-writing-style) — generated JSON regeneratedvar(--accent),var(--text-primary), etc.)text-[Npx]pixel font classes — rem equivalents onlyhandle()and returnIpcResult<T>schema.ts(migration 41 —user_style)electron/db/queries.ts(via re-exporteduser-style-queries.ts)dependencies/devDependencies— none added (N/A)electron/mcp/tools/index.tsdispatch +electron/lib/tool-schemas.tsZod schema (get_user_writing_style)--external:<pkg>flag changes — none (N/A)Notes for reviewer
personalities.json) lives inddutchie/cairn-community— the Browse modal lists entries only once that repo is merged/published.get_user_writing_styleis deliberately lazy (tool-only). It returnsconfigured: falsewhen no style is set up so agents never invent a voice.callLLM); streaming into the preview is a possible follow-up.withPersonality) appends a delimited## Personality: {name}block — never a replacement identity, and community prompts are validated to reject "You are …" openings.Summary by CodeRabbit