Conversation
📝 WalkthroughWalkthroughAdds template-level context configuration (stored as JSON in a new context_option column) and wires it through UI, persistence, and enhancement flow. Introduces a utility to assemble prior-session context from tags, injects contextText into enhancement payloads, and updates the Jinja user template to render it. Includes DB migration and type updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant TemplateEditor as Template Editor (UI)
participant DB as dbCommands (@hypr/plugin-db)
participant Editor as Editor Area
participant Util as prepareContextText
participant Engine as Enhance Engine (Jinja)
User->>TemplateEditor: Edit custom template
TemplateEditor->>DB: listAllTags()
DB-->>TemplateEditor: Tags
TemplateEditor->>TemplateEditor: Set context_option (type, selections)
TemplateEditor->>DB: Save template (context_option)
User->>Editor: Run Enhance with template
Editor->>Editor: Parse template.context_option
alt context type = "tags" with selections
Editor->>Util: prepareContextText(tags, sessionId, userId)
Util->>DB: listAllTags()
DB-->>Util: Tags (with IDs)
loop per tag
Util->>DB: listSessions(tagFilter, tag_id, user_id)
DB-->>Util: Sessions
end
Util-->>Editor: contextText (string)
else no context
Editor-->>Editor: contextText = ""
end
Editor->>Engine: Render enhance.user.jinja with { transcript, participants, contextText? }
Engine-->>Editor: Rendered prompt
Editor-->>User: Enhancement result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks (1 warning, 2 inconclusive)❌ Failed checks (1 warning, 2 inconclusive)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. 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: 3
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/editor-area/index.tsx (1)
500-508: Condition always true; always passes contextText (even when empty)Using OR makes the guard tautological. Pass contextText only when non-empty.
- ...((contextText !== "" || contextText !== undefined || contextText !== null) ? { contextText } : {}), + ...(contextText && contextText.trim().length > 0 ? { contextText } : {}),
🧹 Nitpick comments (11)
crates/template/assets/enhance.user.jinja (1)
15-20: Minor copy tweak: singular “context”Tighten phrasing for clarity.
{% if contextText %} -Below are additional context from previous notes, refer to them if necessary: +Below is additional context from previous notes; refer to it if necessary: <previous_context> {{ contextText }} </previous_context> {% endif %}apps/desktop/src/components/editor-area/index.tsx (1)
451-469: Comment doesn’t match behaviorThe comment says “Silent catch” but we still log. Adjust the comment for accuracy or remove it.
- // Silent catch for malformed JSON + // Ignore parse errors; log and continueapps/desktop/src/components/editor-area/utils/summary-prepare.ts (3)
30-31: Type the sessions map to prevent implicit-any bleed-through.Adds clarity and catches shape drift at compile time.
import { commands as dbCommands } from "@hypr/plugin-db"; +// Derive the session item type from the db command +type Session = Awaited<ReturnType<typeof dbCommands.listSessions>>[number]; + ... - const allSessions = new Map(); + const allSessions = new Map<string, Session>();
32-53: Fetch per-tag sessions in parallel to reduce latency.Current loop awaits sequentially; parallelizing keeps behavior (including per-tag logging) and improves responsiveness with many tags.
- for (const tagId of tagIds) { - try { - const sessions = await dbCommands.listSessions({ - type: "tagFilter", - tag_ids: [tagId], - user_id: userId, - limit: 10, - }); - - sessions.forEach(session => { - if ( - session.id !== currentSessionId - && session.enhanced_memo_html - && session.enhanced_memo_html.trim().length > 0 - ) { - allSessions.set(session.id, session); - } - }); - } catch (error) { - console.warn(`Failed to fetch sessions for tag ${tagId}:`, error); - } - } + await Promise.all( + tagIds.map(async (tagId) => { + try { + const sessions = await dbCommands.listSessions({ + type: "tagFilter", + tag_ids: [tagId], + user_id: userId, + limit: 10, + }); + sessions.forEach((session) => { + if ( + session.id !== currentSessionId && + session.enhanced_memo_html && + session.enhanced_memo_html.trim().length > 0 + ) { + allSessions.set(session.id, session); + } + }); + } catch (error) { + console.warn(`Failed to fetch sessions for tag ${tagId}:`, error); + } + }), + );
63-71: Preserve line breaks and decode common entities for better readability.Plain tag stripping collapses paragraphs/lists; add minimal newline handling and decode quotes.
const contextParts = sortedSessions.map((session, index) => { const cleanContent = session.enhanced_memo_html! - .replace(/<[^>]*>/g, "") + .replace(/<(br|p|\/p|li|\/li)[^>]*>/gi, "\n") + .replace(/<[^>]*>/g, "") .replace(/ /g, " ") .replace(/&/g, "&") .replace(/</g, "<") .replace(/>/g, ">") + .replace(/"/g, '"') + .replace(/'|'/g, "'") + .replace(/\n{3,}/g, "\n\n") .trim();apps/desktop/src/components/settings/views/template.tsx (6)
88-91: Remove debug logs.Avoids noisy console output in production.
- console.log("now in template editor"); - console.log("template: ", template); - console.log("isBuiltinTemplate: ", isBuiltinTemplate);
118-125: Harden parsing to ensure selections is string[].Prevents unexpected shapes from leaking into state.
const parseContextOption = (contextOption: string | null) => { if (!contextOption) { return { type: "", selections: [] }; } try { const parsed = JSON.parse(contextOption); - return { - type: parsed.type || "", - selections: parsed.selections || [], - }; + const selections = + Array.isArray(parsed.selections) + ? parsed.selections.filter((s: unknown): s is string => typeof s === "string") + : []; + return { type: parsed.type || "", selections }; } catch { return { type: "", selections: [] }; } };
337-353: Add aria-label to the remove-tag icon button.Improves accessibility for screen readers.
<Button type="button" variant="ghost" size="sm" className="h-3 w-3 p-0 hover:bg-transparent ml-0.5" + aria-label={t`Remove tag`} onClick={() => { const newSelectedTags = selectedTags.filter((t) => t !== tag); setSelectedTags(newSelectedTags);
365-372: Add aria-label to the add-tag icon button.Improves accessibility for screen readers.
<Button type="button" variant="outline" size="icon" className="h-[38px] w-[38px]" + aria-label={t`Add tag`} > <Plus className="h-4 w-4" />
376-381: Localize static UI strings in the tag picker.Aligns with existing i18n usage in this component.
- <CommandInput placeholder="Search tags..." className="h-9" /> + <CommandInput placeholder={t`Search tags...`} className="h-9" /> <CommandEmpty> - {allTags.length === 0 - ? "No tags available. Create tags by tagging your notes first." - : "No tag found."} + {allTags.length === 0 + ? <Trans>No tags available. Create tags by tagging your notes first.</Trans> + : <Trans>No tag found.</Trans>} </CommandEmpty>
78-83: Trim “what” comments; keep only “why” where helpful.Reduces noise per project guidelines.
Also applies to: 108-111, 142-146, 274-281
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/desktop/src/locales/en/messages.pois excluded by!**/*.poapps/desktop/src/locales/ko/messages.pois excluded by!**/*.poplugins/db/js/bindings.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (10)
apps/desktop/src/components/editor-area/index.tsx(3 hunks)apps/desktop/src/components/editor-area/utils/summary-prepare.ts(1 hunks)apps/desktop/src/components/settings/views/template.tsx(4 hunks)apps/desktop/src/components/settings/views/templates.tsx(1 hunks)apps/desktop/src/utils/default-templates.ts(13 hunks)crates/db-user/src/lib.rs(2 hunks)crates/db-user/src/templates_migration_1.sql(1 hunks)crates/db-user/src/templates_ops.rs(3 hunks)crates/db-user/src/templates_types.rs(2 hunks)crates/template/assets/enhance.user.jinja(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
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/editor-area/utils/summary-prepare.tsapps/desktop/src/utils/default-templates.tscrates/db-user/src/templates_types.rscrates/db-user/src/lib.rsapps/desktop/src/components/settings/views/templates.tsxapps/desktop/src/components/editor-area/index.tsxcrates/db-user/src/templates_ops.rsapps/desktop/src/components/settings/views/template.tsx
🧬 Code graph analysis (4)
apps/desktop/src/components/editor-area/utils/summary-prepare.ts (1)
apps/admin/src/lib/db/schema/auth.ts (1)
session(13-23)
crates/db-user/src/templates_types.rs (1)
crates/db-user/src/sessions_ops.rs (1)
row(281-281)
apps/desktop/src/components/editor-area/index.tsx (1)
apps/desktop/src/components/editor-area/utils/summary-prepare.ts (1)
prepareContextText(10-76)
apps/desktop/src/components/settings/views/template.tsx (4)
packages/ui/src/components/ui/select.tsx (1)
Select(174-174)packages/ui/src/components/ui/badge.tsx (1)
Badge(60-60)packages/ui/src/components/ui/popover.tsx (1)
Popover(85-85)packages/ui/src/components/ui/command.tsx (5)
Command(200-200)CommandInput(204-204)CommandEmpty(202-202)CommandGroup(203-203)CommandItem(205-205)
⏰ 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: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (8)
apps/desktop/src/components/settings/views/templates.tsx (1)
130-139: Default new template includes context_option: null — goodInitializing as null is consistent with defaults and avoids ambiguous empty-string states.
crates/db-user/src/templates_migration_1.sql (1)
1-4: Migration adds context_option column — OKAppend-only and nullable as expected. No further changes needed here.
crates/db-user/src/lib.rs (1)
132-159: Migration wiring appended correctlyLength bumped and new migration placed at the end of the list.
apps/desktop/src/utils/default-templates.ts (1)
18-19: Consistently setting context_option: null across defaults — goodKeeps the shape uniform and interoperates cleanly with the editor logic.
Also applies to: 34-35, 50-51, 67-68, 84-85, 101-102, 118-119, 135-136, 152-153, 169-170, 187-187, 203-204, 220-221
crates/db-user/src/templates_types.rs (1)
11-12: Template gains context_option — goodPublic API matches the new column and UI usage.
apps/desktop/src/components/editor-area/index.tsx (1)
30-31: New utility import — OKUsed below; no unused imports.
crates/db-user/src/templates_ops.rs (1)
103-106: Test case covers tag-based JSON — OKExercising upsert with a non-null context_option looks good.
apps/desktop/src/components/settings/views/template.tsx (1)
287-305: Verify Select placeholder behavior when value is empty.With value set to "", some Select implementations won’t show the placeholder and can emit “value not in items” warnings. Consider mapping empty to undefined or using "none" as the controlled value; verify in the running app.
| tags, | ||
| context_option | ||
| ) VALUES ( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Store NULL, not empty string, for absent context_option
Empty strings complicate semantics across layers. Persist NULLs and keep reads simple. Backward-compatible, especially with the read-side normalization suggested in templates_types.rs.
Apply:
) VALUES (
:id,
:user_id,
:title,
:description,
:sections,
- :tags,
- :context_option
+ :tags,
+ NULLIF(:context_option, '')
) ON CONFLICT(id) DO UPDATE SET
title = :title,
description = :description,
sections = :sections,
- tags = :tags,
- context_option = :context_option
+ tags = :tags,
+ context_option = NULLIF(:context_option, '')Optionally, once this is in place you can later simplify the TS checks to a direct truthy test without worrying about empty strings.
Also applies to: 41-43, 47-49, 57-57
🤖 Prompt for AI Agents
In crates/db-user/src/templates_ops.rs around lines 33-35 (and similarly at
41-43, 47-49, 57-57), the INSERT currently writes an empty string for absent
context_option; change the code to insert SQL NULL instead of an empty string
when context_option is None/absent. Update the parameter binding logic so that
if the Rust value is Option::None you bind a NULL (e.g., use the DB client's
null/Option-aware binding or map to Option::<String>::None) and if Some(v) bind
the string; ensure the SQL VALUES list and prepared parameters align with this
change so reads will return NULL rather than an empty string.
| context_option: row.get(6).ok(), | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize empty string to None when reading context_option
Rows saved with empty-string values will deserialize as Some(""), which is semantically “unset”. Normalize to None for consistency and simpler client checks.
Apply:
- context_option: row.get(6).ok(),
+ context_option: row
+ .get_str(6)
+ .and_then(|s| {
+ let t = s.trim();
+ if t.is_empty() { None } else { Some(t.to_string()) }
+ }),Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/db-user/src/templates_types.rs around lines 37 to 38, the current
deserialization uses row.get(6).ok() which yields Some("") for stored empty
strings; update the read to normalize an empty string into None by mapping the
retrieved value: get the value via row.get(6), convert any Err to None, and if
the retrieved string is present but empty return None, otherwise return
Some(the_string). Replace the simple .ok() with this conditional mapping so
clients see None for semantically unset context_option.
| Your job is to write a perfect note based on the above informations. | ||
| Note that above given informations like participants, transcript, etc. are already displayed in the UI, so you don't need to repeat them. | ||
| Note that above given informations like participants, transcript, previous context, etc. are already displayed in the UI, so you don't need to repeat them. | ||
|
|
There was a problem hiding this comment.
Fix grammar in instructions
Use “information” (uncountable) and concise wording.
-Your job is to write a perfect note based on the above informations.
-Note that above given informations like participants, transcript, previous context, etc. are already displayed in the UI, so you don't need to repeat them.
+Your job is to write a perfect note based on the information above.
+Note that participants, transcript, and previous context are already displayed in the UI; do not repeat them.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Your job is to write a perfect note based on the above informations. | |
| Note that above given informations like participants, transcript, etc. are already displayed in the UI, so you don't need to repeat them. | |
| Note that above given informations like participants, transcript, previous context, etc. are already displayed in the UI, so you don't need to repeat them. | |
| Your job is to write a perfect note based on the information above. | |
| Note that participants, transcript, and previous context are already displayed in the UI; do not repeat them. |
🤖 Prompt for AI Agents
In crates/template/assets/enhance.user.jinja around lines 24 to 26, the
instruction text uses the incorrect plural "informations" and is wordy; change
"above informations" to the uncountable "above information" and tighten the
sentence to something concise (e.g., "Your job is to write a perfect note based
on the above information. Note that participants, transcript, previous context,
etc. are already displayed in the UI, so do not repeat them.").
2 participants
No description provided.