Got enhancing working in desktop2 #1596
Conversation
|
@coderabbitai ignore |
|
Note Reviews pausedUse the following commands to manage reviews:
📝 WalkthroughWalkthroughThis PR introduces AI-powered content enhancement for session notes with streaming support. It adds a new Zustand-based AI task store, integrates OpenRouter and OpenAI language model providers, refactors the EnhancedEditor component into a modular structure with streaming and error-handling views, implements an agent-based meeting summary system with structured Markdown output, and updates the Tiptap editor to operate in Markdown mode. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as GenerateButton UI
participant Task as AI Task Store
participant Agent as Language Model Agent
participant Editor as Enhanced Editor
participant View as StreamingView
User->>UI: Click "Regenerate"
UI->>Task: generate(taskId, prompt, onComplete)
Task->>Task: Create AbortController, set status="generating"
Task->>Agent: agent.stream(messages)
Agent->>Task: Stream text chunks
Task->>Task: Accumulate streamedText, collect steps
Task->>Editor: Re-render with isGenerating=true
Editor->>View: Render StreamingView(streamedText)
View->>View: Auto-scroll to bottom
Agent-->>Task: Finish streaming
Task->>Task: onComplete(streamedText)
Task->>Editor: Update enhanced_md via partial row setter
Task->>Task: Set status="success"
User->>UI: Click "Cancel" (during generation)
UI->>Task: cancel(taskId)
Task->>Agent: Abort streaming
Agent-->>Task: AbortError
Task->>Task: Reset state to idle
Task->>Editor: Re-render, status=idle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes The changes span multiple concerns with varied patterns: new Zustand store architecture with complex streaming and agent logic; substantial component refactoring across the editor hierarchy; provider integration requiring conditional branching; and layout restructuring affecting focus and scroll behavior. While individual files have clear intent, the interconnected task state, streaming mechanics, and agent scheduling introduce moderate logic density warranting careful cross-file review. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/main/body/sessions/note-input/index.tsx (1)
87-100: Users can’t trigger “Enhanced” without a transcriptEnhanced tab appears only if hasTranscript, but generation is only reachable on the Enhanced tab. This blocks enhancement for sessions with only raw notes.
Expose Enhanced when generating or when enhanced content exists (and optionally even when raw notes exist). Example:
function useEditorTabs({ sessionId }: { sessionId: string }): EditorView[] { const status = useListener((state) => state.status); const hasTranscript = useHasTranscript(sessionId); + const taskStatus = useAITask((state) => + state.tasks.get(`${sessionId}-enhance`)?.status ?? "idle", + ); + const hasEnhanced = !!persisted.UI.useCell( + "sessions", + sessionId, + "enhanced_md", + persisted.STORE_ID, + ); if (status === "running_active") { return ["raw", "transcript"]; } - if (hasTranscript) { - return ["enhanced", "raw", "transcript"]; - } + if (hasTranscript || taskStatus === "generating" || hasEnhanced) { + return ["enhanced", "raw", ...(hasTranscript ? ["transcript"] : [])]; + } return ["raw"]; }This aligns with the UX flow and avoids dead-ends. As per coding guidelines.
🧹 Nitpick comments (1)
apps/desktop/src/store/zustand/ai-task/tasks.ts (1)
173-186: Success path looks fine; consider storing trimmed text consistentlyYou store fullText on success but trimmed text during streaming. Optional: also store trimmed on success for consistency, while onComplete receives fullText to persist.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.github/actions/pnpm_install/action.yaml(1 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/components/main/body/index.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/floating/generate.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/floating/index.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/index.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/enhanced.tsx(0 hunks)apps/desktop/src/components/main/body/sessions/note-input/enhanced/editor.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/enhanced/streaming.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/index.tsx(4 hunks)apps/desktop/src/components/settings/ai/shared/model-combobox.tsx(2 hunks)apps/desktop/src/contexts/ai-task/enhancing.ts(1 hunks)apps/desktop/src/contexts/ai-task/index.tsx(1 hunks)apps/desktop/src/hooks/useLLMConnection.ts(2 hunks)apps/desktop/src/main.tsx(2 hunks)apps/desktop/src/routes/app/main/_layout.tsx(3 hunks)apps/desktop/src/store/zustand/ai-task/index.ts(1 hunks)apps/desktop/src/store/zustand/ai-task/tasks.ts(1 hunks)apps/desktop/src/types/index.ts(1 hunks)package.json(1 hunks)packages/tiptap/package.json(1 hunks)packages/tiptap/src/editor/index.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/components/main/body/sessions/note-input/enhanced.tsx
🧰 Additional context used
📓 Path-based instructions (1)
apps/desktop/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop/.cursor/rules/style.mdc)
apps/desktop/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the utilitycnimported asimport { cn } from "@hypr/utils"
When usingcnfor Tailwind classNames, always pass an array
Group Tailwind classNames by logical sections when usingcn(split array items by grouping)
Files:
apps/desktop/src/components/main/body/sessions/note-input/enhanced/streaming.tsxapps/desktop/src/components/main/body/sessions/note-input/enhanced/editor.tsxapps/desktop/src/components/main/body/index.tsxapps/desktop/src/components/main/body/sessions/floating/generate.tsxapps/desktop/src/components/settings/ai/shared/model-combobox.tsxapps/desktop/src/components/main/body/sessions/floating/index.tsxapps/desktop/src/routes/app/main/_layout.tsxapps/desktop/src/contexts/ai-task/index.tsxapps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsxapps/desktop/src/components/main/body/sessions/index.tsxapps/desktop/src/main.tsxapps/desktop/src/components/main/body/sessions/note-input/index.tsx
🧬 Code graph analysis (13)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/streaming.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/editor.tsx (1)
packages/tiptap/src/editor/index.tsx (1)
TiptapEditor(12-12)
apps/desktop/src/components/main/body/sessions/floating/generate.tsx (4)
apps/desktop/src/hooks/useLLMConnection.ts (1)
useLanguageModel(11-51)apps/desktop/src/contexts/ai-task/index.tsx (1)
useAITask(28-42)packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/main/body/sessions/floating/shared.tsx (1)
FloatingButton(5-29)
apps/desktop/src/components/main/body/sessions/floating/index.tsx (1)
apps/desktop/src/components/main/body/sessions/floating/generate.tsx (1)
GenerateButton(11-111)
apps/desktop/src/types/index.ts (2)
apps/desktop/src/store/zustand/listener/index.ts (1)
ListenerStore(10-10)apps/desktop/src/store/zustand/ai-task/index.ts (1)
AITaskStore(9-9)
apps/desktop/src/routes/app/main/_layout.tsx (1)
apps/desktop/src/contexts/ai-task/index.tsx (1)
AITaskProvider(9-26)
apps/desktop/src/store/zustand/ai-task/index.ts (1)
apps/desktop/src/store/zustand/ai-task/tasks.ts (3)
TasksState(26-28)TasksActions(30-47)createTasksSlice(64-218)
apps/desktop/src/contexts/ai-task/index.tsx (1)
apps/desktop/src/store/zustand/ai-task/index.ts (2)
AITaskStore(9-9)createAITaskStore(11-15)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx (4)
packages/tiptap/src/editor/index.tsx (1)
TiptapEditor(12-12)apps/desktop/src/contexts/ai-task/index.tsx (1)
useAITask(28-42)apps/desktop/src/components/main/body/sessions/note-input/enhanced/streaming.tsx (1)
StreamingView(8-57)apps/desktop/src/components/main/body/sessions/note-input/enhanced/editor.tsx (1)
EnhancedEditor(7-36)
apps/desktop/src/components/main/body/sessions/index.tsx (5)
apps/desktop/src/components/main/body/index.tsx (1)
StandardTabWrapper(272-293)apps/desktop/src/components/main/body/sessions/floating/index.tsx (1)
FloatingActionButton(10-32)apps/desktop/src/components/main/body/sessions/outer-header/index.tsx (1)
OuterHeader(6-18)apps/desktop/src/components/main/body/sessions/title-input.tsx (1)
TitleInput(6-32)apps/desktop/src/components/main/body/sessions/note-input/index.tsx (1)
NoteInput(14-50)
apps/desktop/src/store/zustand/ai-task/tasks.ts (1)
apps/desktop/src/contexts/ai-task/enhancing.ts (1)
createEnhancingAgent(4-75)
apps/desktop/src/main.tsx (2)
apps/desktop/src/store/zustand/listener/index.ts (1)
createListenerStore(12-17)apps/desktop/src/store/zustand/ai-task/index.ts (1)
createAITaskStore(11-15)
apps/desktop/src/components/main/body/sessions/note-input/index.tsx (2)
apps/desktop/src/contexts/ai-task/index.tsx (1)
useAITask(28-42)apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx (1)
Enhanced(8-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: zizmor
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (31)
.github/actions/pnpm_install/action.yaml (1)
6-6: LGTM! Version bump aligns with root package.json.The pnpm version update to 10.19.0 is consistent with the packageManager field in the root package.json.
apps/desktop/package.json (1)
39-39: LGTM! New dependency supports OpenRouter integration.The addition of
@openrouter/ai-sdk-providerenables the OpenRouter LLM provider functionality implemented inuseLLMConnection.ts.package.json (2)
2-2: LGTM! Package manager version bump is consistent.The pnpm version update aligns with the GitHub Actions configuration change.
4-5: LGTM! Cosmetic reordering.The swap in devDependencies order has no functional impact.
apps/desktop/src/components/settings/ai/shared/model-combobox.tsx (2)
195-195: Verify the keyword filter for potential false positives.The filter excludes models containing "audio", "image", or "code" as substrings. While this likely achieves the intended goal, it could theoretically match unintended model IDs (e.g., a model named "advanced-coder" would be excluded).
Consider whether the filtering logic should be more specific (e.g., matching specific model families or checking against a known list) or if the current substring approach is sufficient for the OpenRouter/OpenAI model naming conventions.
208-208: Stricter tool parameter requirements.The filter now requires both
"tools"AND"tool_choice"insupported_parameters. This is more restrictive than the previous logic which only checked for"tools".Ensure that this stricter requirement aligns with how the application invokes tool-enabled models. If the application always uses
tool_choice, this change is appropriate. Iftool_choiceis optional, this may filter out valid models.apps/desktop/src/hooks/useLLMConnection.ts (2)
2-4: LGTM! Provider imports added.The new imports for OpenAI and OpenRouter providers are correctly added.
27-41: LGTM! Provider integrations follow consistent patterns.The OpenRouter and OpenAI provider branches follow the same pattern as the existing Anthropic integration. Each creates a provider with the API key and returns a language model instance.
packages/tiptap/package.json (1)
39-39: LGTM! Markdown support added with consistent versioning.The
@tiptap/markdowndependency version ^3.7.2 matches all other TipTap extension versions in the package, ensuring compatibility.apps/desktop/src/types/index.ts (2)
4-4: LGTM! Type import for AI task store.The AITaskStore type import is correctly added to support the context extension.
11-11: LGTM! Context extended with AI task store.The aiTaskStore field is added to the Context type, following the same pattern as the existing listenerStore field.
apps/desktop/src/main.tsx (3)
21-21: LGTM! AI task store factory imported.The createAITaskStore import enables instantiation of the AI task store.
25-25: LGTM! AI task store instantiated.The aiTaskStore is created following the same pattern as the listenerStore on line 24.
53-53: LGTM! AI task store wired into router context.The aiTaskStore is correctly injected into the router context, completing the integration and making it available to all routes via the Context type.
apps/desktop/src/routes/app/main/_layout.tsx (1)
5-5: LGTM! Clean integration of AI task provider.The null guard for
aiTaskStoreensures safe rendering, and the provider correctly scopes AI task state within this layout.Also applies to: 20-20, 56-68
apps/desktop/src/components/main/body/sessions/floating/index.tsx (1)
22-22: LGTM! Correct prop passing.The
sessionIdprop correctly ties the generate action to the specific session, aligning with the updatedGenerateButtonsignature.packages/tiptap/src/editor/index.tsx (2)
55-57: Good UX improvement.Adding
autocapitalize="off"improves the editing experience by preventing unwanted auto-capitalization in note-taking contexts.
42-42: Clarify markdown content retrieval inconsistency.The
contentType: 'markdown'configuration controls how input is parsed, not whatgetHTML()returns—getHTML()always returns HTML. Lines 42, 49, and 52 show a potential inconsistency:
- Editor is configured for markdown input with the Markdown extension
handleChangecallseditor.getHTML(), which returns HTMLIf markdown content retrieval is intended, use
editor.getMarkdown()oreditor.markdown.serialize()instead. If HTML output is intentional despite markdown mode, document this design decision and confirm theHTMLContenttype signature aligns with the actual output format.apps/desktop/src/contexts/ai-task/index.tsx (1)
1-42: LGTM! Well-implemented context pattern.The provider correctly memoizes the store via
useRef, and theuseAITaskhook properly applies shallow comparison for selector optimization. Clear error message enforces proper usage within the provider boundary.apps/desktop/src/store/zustand/ai-task/index.ts (1)
1-15: LGTM! Clean store factory pattern.The store composition is straightforward, correctly spreads the tasks slice, and type definitions are clear.
apps/desktop/src/components/main/body/sessions/note-input/enhanced/streaming.tsx (3)
59-82: LGTM! Well-implemented auto-scroll behavior.The hook correctly identifies the scrollable parent and only auto-scrolls when the user is near the bottom (within 100px), preserving user control when they've scrolled up.
29-32: Follow coding guidelines forcn()usage.The
cn()call should receive an array per coding guidelines: "When usingcnfor Tailwind classNames, always pass an array."As per coding guidelines.
Apply this diff:
- className={cn([ - "text-sm leading-relaxed", - "whitespace-pre-wrap break-words", - ])} + className={cn([ + ["text-sm leading-relaxed"], + ["whitespace-pre-wrap break-words"], + ])}⛔ Skipped due to learnings
Learnt from: CR PR: fastrepl/hyprnote#0 File: apps/desktop/.cursor/rules/style.mdc:0-0 Timestamp: 2025-10-21T03:03:20.070Z Learning: Applies to apps/desktop/**/*.{tsx,jsx} : When using `cn` for Tailwind classNames, always pass an arrayLearnt from: CR PR: fastrepl/hyprnote#0 File: apps/desktop/.cursor/rules/style.mdc:0-0 Timestamp: 2025-10-21T03:03:20.070Z Learning: Applies to apps/desktop/**/*.{tsx,jsx} : When there are many Tailwind classNames with conditional logic, use the utility `cn` imported as `import { cn } from "hypr/utils"`Learnt from: CR PR: fastrepl/hyprnote#0 File: apps/desktop/.cursor/rules/style.mdc:0-0 Timestamp: 2025-10-21T03:03:20.070Z Learning: Applies to apps/desktop/**/*.{tsx,jsx} : Group Tailwind classNames by logical sections when using `cn` (split array items by grouping)
46-50: Follow coding guidelines forcn()usage.The
cn()call should receive an array per coding guidelines, with logical grouping of className sections.As per coding guidelines.
Apply this diff:
- className={cn([ - "flex items-center justify-center w-[calc(100%-24px)] gap-3", - "border border-neutral-200", - "bg-neutral-800 rounded-lg py-3", - ])} + className={cn([ + ["flex items-center justify-center w-[calc(100%-24px)] gap-3"], + ["border border-neutral-200"], + ["bg-neutral-800 rounded-lg py-3"], + ])}⛔ Skipped due to learnings
Learnt from: CR PR: fastrepl/hyprnote#0 File: apps/desktop/.cursor/rules/style.mdc:0-0 Timestamp: 2025-10-21T03:03:20.070Z Learning: Applies to apps/desktop/**/*.{tsx,jsx} : Group Tailwind classNames by logical sections when using `cn` (split array items by grouping)Learnt from: CR PR: fastrepl/hyprnote#0 File: apps/desktop/.cursor/rules/style.mdc:0-0 Timestamp: 2025-10-21T03:03:20.070Z Learning: Applies to apps/desktop/**/*.{tsx,jsx} : When using `cn` for Tailwind classNames, always pass an arrayLearnt from: CR PR: fastrepl/hyprnote#0 File: apps/desktop/.cursor/rules/style.mdc:0-0 Timestamp: 2025-10-21T03:03:20.070Z Learning: Applies to apps/desktop/**/*.{tsx,jsx} : When there are many Tailwind classNames with conditional logic, use the utility `cn` imported as `import { cn } from "hypr/utils"`apps/desktop/src/components/main/body/sessions/note-input/enhanced/editor.tsx (1)
1-36: LGTM! Clean enhanced editor implementation.The component correctly reads from and writes to the persisted store, properly forwards the editor ref, and includes a minimal but valid mention configuration.
apps/desktop/src/components/main/body/index.tsx (1)
272-293: LGTM! Clean API extension.The
floatingButtonprop is properly typed and positioned, enabling session-specific floating actions while maintaining the existingTabChatButtonbehavior.apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx (2)
14-21: Selector reads look solidDerived object avoids Map identity pitfalls; good use of a narrow selector.
27-31: Streaming branch is appropriateClean separation: stream view while generating, editor otherwise.
apps/desktop/src/components/main/body/sessions/index.tsx (1)
56-67: Wrapper integration LGTMfloatingButton placement and flex layout changes look correct; no conditional className complexity here.
apps/desktop/src/contexts/ai-task/enhancing.ts (1)
1-75: Agent setup is coherentPrompt, tool schema, and step orchestration look consistent. Biggest risk now is upstream callers not supplying the raw content (addressed in GenerateButton review).
If you want, I can add a quick unit harness to simulate a tool-invocation step with sample content to ensure sections are produced.
apps/desktop/src/components/main/body/sessions/note-input/index.tsx (1)
31-35: Auto-switch-on-generate is goodEnsures users see streaming when a task starts.
apps/desktop/src/components/main/body/sessions/floating/generate.tsx (1)
49-49: Expose generating state clearlyisGenerating derivation is correct and keeps template UI gated while running.
apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx
Show resolved
Hide resolved
✅ Actions performedReviews paused. |
No description provided.