Add llm config error rendered in Editor area #1728
Conversation
The app was hitting React's "Maximum update depth exceeded" when stopping listening due to frequent calls to updateSessionTabState that triggered repeated re-renders. To fix this, replace direct uses of changing props/state in handlers with stable refs and memoized callbacks: useRef is used to hold the current tab/sessionTab in note-input, useAutoEnhance, and useRunBatch, and handleTabChange is wrapped with useCallback. Also remove an unnecessary sessionTab dependency from a hook return to avoid re-triggering updates. These changes prevent excessive state updates and stop creation of many enhanced notes. ㅍ
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe changes introduce LLM connection status monitoring across the enhanced note input component hierarchy. A new ConfigError component renders connection errors with configuration options. Multiple hooks and components are refactored to use stable refs for tab and session state to prevent closure over stale data. Changes
Sequence DiagramsequenceDiagram
participant Enhanced as Enhanced Component
participant LLM as useLLMConnectionStatus Hook
participant ConfigErr as ConfigError Component
participant Nav as Window Navigation
Enhanced->>LLM: Get LLM connection status
LLM-->>Enhanced: Return status (pending/error/connected)
alt Task Idle + Config Error
Enhanced->>ConfigErr: Render with status
ConfigErr-->>Enhanced: Show error message + button
Note over ConfigErr: User clicks Configure
ConfigErr->>Nav: Open settings window
Nav->>Nav: Jump to intelligence tab
else Task Generating
Enhanced->>Enhanced: Render StreamingView
else Normal State
Enhanced->>Enhanced: Render EnhancedEditor
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 0
🧹 Nitpick comments (4)
apps/desktop/src/hooks/useLLMConnection.ts (1)
243-246: ThinuseLLMConnectionStatuswrapper is fine; watch for duplicated work at call sitesThis hook cleanly exposes just the status while reusing
useLLMConnection. If you later have components that need bothstatusandconn, consider callinguseLLMConnectiondirectly there to avoid subscribing twice to the same underlying state.apps/desktop/src/components/main/body/sessions/note-input/header.tsx (1)
21-24: Config‑error awareness inuseEnhanceLogicis good; consider sharing logic and surfacing a messageWiring
llmStatusintouseEnhanceLogicso thatisErroralso covers idle config/auth problems is a good fit with the new editor‑areaConfigError. Two small follow‑ups to consider:
isErrorcan be true viaisIdleWithConfigErrorwhileerrorstaysnull, so the red regenerate icon shows with no tooltip content. You might want to derive a simple message fromllmStatusin that branch (e.g., reuse the same mapping asConfigError) so the tooltip explains what’s wrong.- The
isConfigErrorpredicate is duplicated here and inEnhanced; extracting a tiny helper likeisLLMConfigError(llmStatus)would keep header and editor behavior in lockstep and make it easy to add cases likeprovider_not_foundlater if you decide that should present as a config issue too.Also applies to: 415-478
apps/desktop/src/components/main/body/sessions/note-input/enhanced/config-error.tsx (1)
1-70:ConfigErrorimplementation is clear; consider explicit provider_not_found handling and shared navigation helperThis component cleanly surfaces config/auth problems in the editor with an obvious “Configure” action, which fits the new flow well. Two optional tweaks:
getMessageForStatusdoesn’t special‑caseprovider_not_found, so that scenario would get the generic fallback. If you ever treatprovider_not_foundas a config error, adding a dedicated message here would make debugging mis‑spelled/custom providers easier.- The
handleConfigureClicksequence mirrorshandleGoToTemplatesinheader.tsx(samewindowShow+ timeout +windowEmitNavigatepattern with a differenttabquery). If this pattern spreads further, a small shared helper for “open settings tab X” would reduce duplication and keep timing behavior consistent.apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx (1)
6-8: Editor‑areaConfigErrorgating is implemented well; centralizeisConfigErrorlogicUsing
llmStatusto short‑circuit the Enhanced view with<ConfigError>when the enhance task is idle and the LLM is misconfigured/unauthenticated gives a much clearer UX than silently doing nothing. Since theisConfigErrorcondition here matches the one inuseEnhanceLogic, consider extracting a shared helper (or exposing anisConfigErrorboolean fromuseLLMConnectionStatus) so behavior stays consistent and it’s easy to expand the set of “config error” reasons (likeprovider_not_found) in one place.Also applies to: 17-28
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/config-error.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx(2 hunks)apps/desktop/src/components/main/body/sessions/note-input/header.tsx(3 hunks)apps/desktop/src/components/main/body/sessions/note-input/index.tsx(2 hunks)apps/desktop/src/hooks/useAutoEnhance.ts(3 hunks)apps/desktop/src/hooks/useLLMConnection.ts(2 hunks)apps/desktop/src/hooks/useRunBatch.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: plyght
Repo: fastrepl/hyprnote PR: 921
File: plugins/location-connectivity/src/error.rs:1-41
Timestamp: 2025-06-06T16:31:46.457Z
Learning: In the location-connectivity plugin (plugins/location-connectivity/), comprehensive error handling via LocationConnectivityError is necessary despite the "no error handling" guideline, as the plugin performs system-level operations (WiFi detection, persistent storage) that require proper error propagation to the frontend.
🧬 Code graph analysis (4)
apps/desktop/src/components/main/body/sessions/note-input/index.tsx (1)
apps/desktop/src/store/zustand/tabs/schema.ts (1)
EditorView(18-18)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/config-error.tsx (1)
apps/desktop/src/hooks/useLLMConnection.ts (1)
LLMConnectionStatus(29-40)
apps/desktop/src/components/main/body/sessions/note-input/header.tsx (1)
apps/desktop/src/hooks/useLLMConnection.ts (1)
useLLMConnectionStatus(243-246)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx (3)
apps/desktop/src/hooks/useLLMConnection.ts (1)
useLLMConnectionStatus(243-246)apps/desktop/src/hooks/useAITaskTask.ts (1)
useAITaskTask(39-105)apps/desktop/src/components/main/body/sessions/note-input/enhanced/config-error.tsx (1)
ConfigError(8-41)
⏰ 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: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (6)
apps/desktop/src/hooks/useRunBatch.ts (3)
1-1: LGTM: Import addition is correct.The
useRefimport is necessary for the ref-based stability pattern implemented below.
37-38: LGTM: Ref pattern correctly prevents stale closures.The ref is properly initialized and kept synchronized with
sessionTabon every render. This ensures the memoized callback always accesses the latest tab state without requiringsessionTabin the dependency array, maintaining stable callback identity.
68-72: LGTM: Ref usage is correct and safe.The callback correctly reads the latest
sessionTabvalue viasessionTabRef.current, includes a null check for safety, and the dependency array properly excludessessionTabsince it's accessed through the ref.apps/desktop/src/hooks/useLLMConnection.ts (1)
29-40: ExportingLLMConnectionStatusunion looks correct and future‑proofThe union variants align with all branches in
useLLMConnection, so making this type public is safe and enables consistent status handling across consumers.apps/desktop/src/hooks/useAutoEnhance.ts (1)
36-38: tabRef + deferred model requirement make auto‑enhance more robustUsing
tabRef.currentinside the memoizedcreateAndStartEnhanceis a good way to avoid stale tab closures while keepinghandleTabChange‑style callbacks stable. Dropping the immediatemodelprecondition and relying on theuseEffectthat checksautoEnhancedNoteId && model && !startedTasksRef.current.has(...)means the enhanced note is created as soon as there’s a transcript, and the task starts once an LLM is configured, which matches the new config‑error UX.To be safe, please manually verify:
- Recordings that finish while LLM config is missing create exactly one enhanced note per session.
- Once the user configures the LLM, those pending notes start and complete enhancement without needing another recording.
Also applies to: 58-71
apps/desktop/src/components/main/body/sessions/note-input/index.tsx (1)
1-1: StabilizinghandleTabChangewithtabRefis a solid cleanupMemoizing
handleTabChangeand readingtabRef.currentavoids stale tab issues in keyboard shortcuts while preventing unnecessary re‑bindings; this is a nice consistency improvement with the other tabRef usages in the PR.Also applies to: 31-39
No description provided.