-
Notifications
You must be signed in to change notification settings - Fork 418
add automatic title generation #935
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis update introduces automatic title generation for meeting notes by integrating a new AI-powered mutation that triggers after content enhancement. Supporting changes include new grammar and template files for title generation, updates to the backend to select grammars dynamically, and test coverage for title generation logic. Minor refactoring and metadata adjustments are also present. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditorArea
participant AIEnhanceMutation
participant AITitleMutation
participant SessionStore
User->>EditorArea: Trigger content enhancement
EditorArea->>AIEnhanceMutation: Call enhance mutation
AIEnhanceMutation->>AIEnhanceMutation: Enhance content (AI)
AIEnhanceMutation-->>EditorArea: onSuccess(enhancedContent)
EditorArea->>AITitleMutation: Call title generation mutation (enhancedContent)
AITitleMutation->>AITitleMutation: Generate title (AI)
AITitleMutation-->>SessionStore: Update session title (if not already set)
sequenceDiagram
participant Frontend
participant Backend (local-llm)
participant GBNF
participant TemplateEngine
Frontend->>Backend (local-llm): Request title generation with metadata (grammar: "title")
Backend (local-llm)->>GBNF: Select Title grammar (if grammar: "title")
Backend (local-llm)->>TemplateEngine: Render system/user title templates
Backend (local-llm)->>AI Model: Generate title with selected grammar and prompt
AI Model-->>Backend (local-llm): Return generated title
Backend (local-llm)-->>Frontend: Return title
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (7)
crates/template/assets/create_title.user.jinja (1)
1-5: Template looks fine but drop superfluous wrapper tagsThe
<enhanced_note>wrapper isn’t referenced by the downstream prompt logic and slightly pollutes the token budget.
Unless a parser relies on those tags, consider omitting them:-<enhanced_note> -{{ enhanced_note }} -</enhanced_note> +{{ enhanced_note }}apps/desktop/src/components/editor-area/note-header/index.tsx (1)
15-17: Provide sane defaults for new optional props
isGeneratingTitleprop may reach child components asundefined. Giving it an explicit default (false) avoids implicit coercion and narrows the type consumers need to handle.interface NoteHeaderProps { ... onGenerateTitle?: () => void; - isGeneratingTitle?: boolean; + isGeneratingTitle?: boolean | undefined; } export function NoteHeader( - { onNavigateToEditor, editable, sessionId, hashtags = [], onGenerateTitle, isGeneratingTitle }: NoteHeaderProps, + { + onNavigateToEditor, + editable, + sessionId, + hashtags = [], + onGenerateTitle, + isGeneratingTitle = false, + }: NoteHeaderProps, ) {crates/gbnf/src/lib.rs (1)
26-41: Consider edge-case testsCurrent tests verify basic happy-/sad-paths. Titles with multiple consecutive spaces or trailing spaces are untested and could surface parsing quirks.
apps/desktop/src/components/editor-area/note-header/title-input.tsx (1)
31-57: Expose generation state for a11yAdding
aria-busy(oraria-live) to the button while generation is active improves screen-reader feedback.- <button + <button + aria-busy={isGeneratingTitle} onClick={onGenerateTitle}plugins/local-llm/src/lib.rs (2)
112-136: Avoid redundantuseinside fnImports inside
title_generation_requestduplicate module-level imports and slightly bloat build times. Pull them to the file header or use fully-qualified paths.
211-296: Tests share duplicated logicBoth streaming and non-streaming title tests duplicate server bootstrapping and assertions. Extract a helper to DRY them up and speed up future additions.
-fn run_title_test(stream: bool) -> anyhow::Result<()> { ... } - -#[tokio::test] async fn test_title_generation_non_streaming() { run_title_test(false).await? } -#[tokio::test] async fn test_title_generation_streaming() { run_title_test(true).await? }apps/desktop/src/components/editor-area/index.tsx (1)
91-101: Avoid duplicating responsibility between hook-level and call-site callbacks.
useGenerateTitleMutationalready contains anonSuccessthat logs analytics. The inlineonSuccesshere adds onlyupdateTitle. Moving that responsibility into the hook (by acceptingupdateTitleas an option) would:
- Keep all side-effects in one place.
- Prevent accidental divergence if other callers forget to pass the callback.
Not critical, but streamlines the API.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/desktop/src/components/editor-area/index.tsx(4 hunks)apps/desktop/src/components/editor-area/note-header/index.tsx(2 hunks)apps/desktop/src/components/editor-area/note-header/title-input.tsx(2 hunks)apps/desktop/src/locales/en/messages.po(1 hunks)apps/desktop/src/locales/ko/messages.po(1 hunks)crates/gbnf/assets/title.gbnf(1 hunks)crates/gbnf/src/lib.rs(1 hunks)crates/template/assets/create_title.user.jinja(1 hunks)crates/template/src/lib.rs(3 hunks)plugins/local-llm/src/lib.rs(2 hunks)plugins/local-llm/src/server.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{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".
**/*.{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".
plugins/local-llm/src/lib.rsapps/desktop/src/components/editor-area/note-header/title-input.tsxapps/desktop/src/components/editor-area/note-header/index.tsxplugins/local-llm/src/server.rscrates/template/src/lib.rsapps/desktop/src/components/editor-area/index.tsxcrates/gbnf/src/lib.rs
🧬 Code Graph Analysis (2)
plugins/local-llm/src/lib.rs (3)
packages/client/generated/types.gen.ts (2)
CreateChatCompletionRequest(238-385)ChatCompletionRequestMessage(101-172)plugins/db/src/lib.rs (1)
create_app(98-103)plugins/local-llm/src/ext.rs (2)
api_base(12-12)api_base(39-43)
crates/template/src/lib.rs (1)
plugins/db/js/bindings.gen.ts (1)
Template(162-162)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci
🔇 Additional comments (8)
apps/desktop/src/locales/ko/messages.po (1)
1075-1077: No action neededOnly the source-reference line number changed; the msgid/msgstr pair is untouched.
LGTM.apps/desktop/src/locales/en/messages.po (1)
1075-1077: No action neededSame as the KO catalogue: just a reference-line update.
apps/desktop/src/components/editor-area/note-header/index.tsx (1)
41-43: 👍 Props correctly forwarded toTitleInput
New props are relayed without breaking existing API, keeping the component tree flat.crates/gbnf/src/lib.rs (1)
3-16: Enum growth handled cleanlyAdding the
TITLEconstant andTitlevariant keeps the factory method exhaustive and future-proof. No issues spotted.apps/desktop/src/components/editor-area/note-header/title-input.tsx (1)
2-3: Ensure dependency is declared
lucide-reactwas introduced; check that the package is added to allpackage.jsonworkspaces to avoid runtime resolution failures.crates/template/src/lib.rs (2)
37-41: 👍 PredefinedTemplate variants added coherentlyEnum serialization keys keep the existing naming convention (
kebab.case), preventing template-lookup regressions.
75-84: 🛠️ Refactor suggestionFail fast with context
init()silently panics on template-registration failure (unwrap). Returning aResultpropagates the error upward and eases diagnostics in integration environments.-pub fn init(env: &mut minijinja::Environment) { +pub fn init(env: &mut minijinja::Environment) -> Result<(), TemplateError> { ... - env.add_template(...).unwrap(); + env.add_template(...).map_err(|e| TemplateError::from(e))?; ... - env.add_template(...).unwrap(); + env.add_template(...).map_err(|e| TemplateError::from(e))?; ... + Ok(()) }Likely an incorrect or invalid review comment.
apps/desktop/src/components/editor-area/index.tsx (1)
128-130: Prop wiring LGTM.
onGenerateTitleandisGeneratingTitleare correctly forwarded toNoteHeader; naming is clear and the loading flag matches React-Query’s"pending"state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/gbnf/assets/title.gbnf (1)
1-3: Trailing–space concern fully resolved—nice!
root ::= start char* (" " char+)*now requires a non-space character after every space, eliminating the “Project” edge case flagged earlier.
No further issues spotted in the grammar.
🧹 Nitpick comments (1)
crates/template/assets/create_title.system.jinja (1)
1-2: Mirror the exact character rules in the system promptFor maximum steerability (and to reinforce the user-prompt change), echo the hard constraints here as well:
-You are a professional assistant that generates a perfect title for a meeting note in {{ config.general.display_language | language }}. -Only output title, nothing else. +You are a professional assistant that generates a perfect title (title case) for a meeting note in {{ config.general.display_language | language }}. +Constraints: title must start with an uppercase letter or digit and contain only letters, digits, and single spaces—no punctuation. +Output only the title text, nothing else.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/desktop/src/components/editor-area/index.tsx(6 hunks)apps/desktop/src/components/editor-area/note-header/index.tsx(1 hunks)crates/gbnf/assets/title.gbnf(1 hunks)crates/gbnf/src/lib.rs(1 hunks)crates/template/assets/create_title.system.jinja(1 hunks)crates/template/assets/create_title.user.jinja(1 hunks)packages/utils/src/ai.ts(1 hunks)plugins/local-llm/src/lib.rs(3 hunks)plugins/local-llm/src/server.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/utils/src/ai.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/desktop/src/components/editor-area/note-header/index.tsx
- plugins/local-llm/src/lib.rs
- crates/gbnf/src/lib.rs
- plugins/local-llm/src/server.rs
- apps/desktop/src/components/editor-area/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci
- GitHub Check: ci (macos, macos-latest)
No description provided.