Conversation
…ix-korean-independence
📝 WalkthroughWalkthroughIntroduces a gating check before auto-generating session tags by fetching existing tags first; removes STT model selection mutations in the welcome modal in favor of local state transitions; updates the displayed STT model size label; adjusts source line refs in English and Korean PO files. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as EditorArea
participant DB as dbCommands
participant TG as AutoTagGenerator
participant QS as QueryStore
UI->>DB: listSessionTags(sessionId)
DB-->>UI: sessionTags[]
UI->>UI: if (hasSessionState && sessionTags.length < 2)
alt Gate passes
UI->>TG: generateTagsForSession(sessionId)
TG-->>UI: tags upserted/assigned
UI->>QS: invalidate tag queries
else Gate fails
UI->>UI: skip auto-tag
end
sequenceDiagram
autonumber
participant U as User
participant WM as WelcomeModal
participant LS as Local State
participant NAV as Navigator
U->>WM: Choose/confirm STT model
WM->>LS: set selectedSttModel
WM->>LS: dismiss model download toast
WM->>NAV: navigate to download-progress
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
cubic analysis
No issues found across 5 files. Review in cubic
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/desktop/src/components/welcome-modal/download-progress-view.tsx (1)
233-239: Avoid hardcoding STT size; derive from selected model (and keep unit formatting consistent).Right now the size label is hardcoded to "264 MB". Since selectedSttModel is a prop, prefer deriving the size based on the actual model to avoid drift. Also note the inconsistency between "264 MB" (with space) and "1.1GB" (without space) in the LLM card.
- Optional refactor: compute the size label from the model (map or metadata) and pass it here.
- Nitpick: standardize units ("MB", "GB") spacing across cards.
Example change within this block:
- size={"264 MB"} + size={getSttSizeLabel(selectedSttModel)}Outside this block, add a helper (adjust to your data source):
// module scope function getSttSizeLabel(model: WhisperModel): string { // Prefer fetching metadata from localSttCommands if available; otherwise, map what you support today. switch (model) { case "QuantizedSmall": return "264 MB"; default: return "—"; // Unknown or not applicable } }And consider aligning the LLM size label to "1.1 GB" for consistency.
apps/desktop/src/components/editor-area/index.tsx (2)
69-71: Tighten comment to explain “why,” not “what.”Current: “check and run auto tag generation if the session has less than 2 tags.”
Suggested: “Gate auto-tagging to avoid over-tagging; only suggest tags if a session currently has <2.”
- // check and run auto tag generation if the session has less than 2 tags + // Gate auto-tagging to avoid over-tagging: run only when the session has <2 tags
82-97: Skip assigning tags already attached to the session.If a suggested tag already exists on this session, assignTagToSession may error or create duplicates depending on DB constraints. Use the fetched sessionTags to skip duplicates case-insensitively.
- for (const tagName of suggestedTags.slice(0, 2)) { + const existingSessionTagNames = new Set( + sessionTags.map((t: { name: string }) => t.name.toLowerCase()) + ); + for (const tagName of suggestedTags.slice(0, 2)) { try { const existingTag = existingTagsMap.get(tagName.toLowerCase()); const tag = await dbCommands.upsertTag({ id: existingTag?.id || crypto.randomUUID(), name: tagName, }); - await dbCommands.assignTagToSession(tag.id, targetSessionId); + if (!existingSessionTagNames.has(tag.name.toLowerCase())) { + await dbCommands.assignTagToSession(tag.id, targetSessionId); + } } catch (error) { console.error(`Failed to assign tag "${tagName}":`, error); } }apps/desktop/src/components/welcome-modal/index.tsx (3)
77-82: Remove or annotate commented-out mutation block.The selectSTTModel useMutation is fully commented. If this is intentional and temporary, replace with a brief “why” comment (e.g., “Disabled to avoid remote mutation; model is set post-download in DownloadProgressView”). Otherwise, drop the dead code.
- /* - const selectSTTModel = useMutation({ - mutationFn: (model: WhisperModel) => localSttCommands.setCurrentModel(model), - }); - */ + // Intentionally disabled: avoid remote mutation here; we set the model after download in DownloadProgressView.
268-276: Ensure STT model selection is persisted even if the modal closes before downloads complete.You now skip setting the STT model here (by design), relying on DownloadProgressView to call setCurrentModel after STT finishes downloading. If the user closes the modal early and downloads finish in the background, setCurrentModel might never run.
Optional: on modal close, if the target model is already downloaded, persist the selection then. This keeps state consistent while preserving your new flow.
Outside this block, inside checkAndShowToasts(), after verifying the model exists:
// After: const sttModelExists = await localSttCommands.isModelDownloaded(selectedSttModel) if (sttModelExists) { await localSttCommands.setCurrentModel(selectedSttModel); }Also, consider adjusting the inline comments (“Automatically select the 'small' model…”) to capture the “why” (faster downloads and lower friction) per the comment guideline.
280-286: Same as above for BYOM path.Mirror the persistence fallback for the BYOM/custom-endpoint path on modal close so the selected STT model is set if already downloaded.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/desktop/src/components/editor-area/index.tsx(1 hunks)apps/desktop/src/components/welcome-modal/download-progress-view.tsx(1 hunks)apps/desktop/src/components/welcome-modal/index.tsx(4 hunks)apps/desktop/src/locales/en/messages.po(1 hunks)apps/desktop/src/locales/ko/messages.po(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/welcome-modal/download-progress-view.tsxapps/desktop/src/components/editor-area/index.tsxapps/desktop/src/components/welcome-modal/index.tsx
⏰ 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). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (3)
apps/desktop/src/locales/ko/messages.po (1)
447-449: LGTM: Only source reference moved for "Back".No functional change. The location annotation shift looks correct and consistent with the Welcome Modal edits.
apps/desktop/src/locales/en/messages.po (1)
447-449: LGTM: Only source reference moved for "Back".No functional change. Mirrors the ko locale update.
apps/desktop/src/components/editor-area/index.tsx (1)
69-73: listSessionTags expects a positional sessionId — original concern is incorrectDefinition found:
- plugins/db/js/bindings.gen.ts:136–137 — async listSessionTags(sessionId: string): Promise<Tag[]> { return await TAURI_INVOKE("plugin:db|list_session_tags", { sessionId }); }
Call sites (using positional string arg):
- apps/desktop/src/components/editor-area/index.tsx:70
- apps/desktop/src/components/toolbar/buttons/share-button.tsx:65
- apps/desktop/src/components/editor-area/note-header/chips/tag-chip.tsx:23, 83, 228
- apps/desktop/src/utils/tag-generation.ts:24, 90
Conclusion: calling dbCommands.listSessionTags(targetSessionId) is correct; no change required.
Likely an incorrect or invalid review comment.
Summary by cubic
Hotfix to stabilize onboarding and reduce tag noise. Only generate tags when a session has fewer than two, show the correct 264 MB STT download size, and skip a failing model mutation so downloads start reliably.