Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR introduces language-aware system prompts by threading language preference through the task processing pipeline. It extracts language from application state, renders system prompts dynamically via template commands with language context, and passes the resulting system prompt to the chat transport for agent configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Settings
participant Store as App Store
participant ChatSession as Chat Session Component
participant TemplateCmd as Template Commands
participant Transport as Chat Transport
participant Agent as AI Agent
User->>Store: Select Language (e.g., "es")
ChatSession->>Store: Read ai_language preference
Store-->>ChatSession: Return language code
ChatSession->>TemplateCmd: render("chat.system", { language: "es" })
Note over TemplateCmd: enforce_language("es", "IMPORTANT: Respond in")<br/>injects language directive
TemplateCmd-->>ChatSession: Return system prompt with<br/>language enforcement
ChatSession->>Transport: new CustomChatTransport(registry, model, systemPrompt)
Transport->>Agent: Configure agent with system prompt<br/>(includes language directive)
Note over Agent: System prompt now constrains<br/>agent responses to selected language
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)
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 (5)
apps/desktop/src/components/settings/general/main-language.tsx (1)
17-20: Consider usingISO_639_1_CODEfor thevalueprop type.The
valueprop is typed asstringwhilesupportedLanguagesis typed asISO_639_1_CODE[]. Using the same type forvaluewould provide better type safety and ensure only valid language codes are passed.}: { - value: string; - onChange: (value: string) => void; + value: ISO_639_1_CODE; + onChange: (value: ISO_639_1_CODE) => void; supportedLanguages: ISO_639_1_CODE[]; }) {apps/desktop/src/components/chat/session.tsx (2)
167-179: Guard async systemPrompt loading to avoid stale updates / unmounted setStateThe effect correctly reloads the system prompt when
languagechanges, but it can still resolve after the language has changed again or the component has unmounted. A small guard improves robustness and avoids stalesystemPromptupdates:- useEffect(() => { - templateCommands - .render("chat.system", { language }) - .then((result) => { - if (result.status === "ok") { - setSystemPrompt(result.data); - } - }) - .catch(console.error); - }, [language]); + useEffect(() => { + let cancelled = false; + + templateCommands + .render("chat.system", { language }) + .then((result) => { + if (!cancelled && result.status === "ok") { + setSystemPrompt(result.data); + } + }) + .catch((error) => { + if (!cancelled) { + console.error(error); + } + }); + + return () => { + cancelled = true; + }; + }, [language]);This ensures older in‑flight renders don’t overwrite a newer language’s prompt and avoids calling
setSystemPromptafter unmount.Please confirm in the
@hypr/plugin-templatedocs thatrender("chat.system", { language })is side‑effect free and safe to retry, so this cancellation pattern won’t interfere with internal caching or metrics.
186-187: Transport memoization with systemPrompt is correct; consider gating on prompt readinessIncluding
systemPromptin theuseMemodependency and passing it intoCustomChatTransportensures language/template changes take effect for subsequent messages, which matches the PR’s goal.If you want to guarantee that every message in a session uses a resolved system prompt (and never falls back to
undefinedon first render), consider delaying transport creation until the prompt is ready, e.g.:- if (!model) { + if (!model) { return null; } - - return new CustomChatTransport(registry, model, systemPrompt); - }, [registry, model, systemPrompt]); + if (!systemPrompt) { + return null; + } + + return new CustomChatTransport(registry, model, systemPrompt); + }, [registry, model, systemPrompt]);That’s optional and depends on whether “no system prompt until template loads” is acceptable for the first user message.
It may be worth confirming in the
@ai-sdk/reactdocs that swapping thetransportinstance during a session (whensystemPromptappears) is fully supported for youruseChatusage.apps/desktop/src/store/zustand/ai-task/task-configs/title-transform.ts (1)
12-15: Language derivation for title looks good; consider de‑duplicating helperReading
ai_languagefrom the store with a non‑empty check and defaulting to"en"is sane and gives the template a stable language value. The samegetLanguagelogic exists inenhance-transform.ts; consider extracting a tiny shared helper so the behavior can’t drift between title and enhance over time (optional).Also applies to: 22-25
crates/template/assets/_language.partial.jinja (1)
1-5: Macro logic is correct; consider language name for clarity.The conditional logic and whitespace handling are sound. The macro correctly skips enforcement for English and empty values.
However, the output format uses language codes directly (e.g., "Generate the title in ko language."), which is grammatically awkward. Consider one of these alternatives for better readability:
- Option 1: "Generate the title in {language_code}." (simpler)
- Option 2: Map codes to names (e.g., "ko" → "Korean") for grammatically correct output
Since LLMs can likely understand language codes in context, this is optional.
Optional: Alternative with cleaner formatting
{% macro enforce_language(language_code, prefix, suffix="language.") -%} {% if language_code and language_code != "en" -%} -{{ prefix }} {{ language_code }} {{ suffix }} +{{ prefix }} {{ language_code }}. {% endif -%} {%- endmacro %}This produces: "Generate the title in ko." which reads more naturally.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/desktop/src/chat/transport.ts(2 hunks)apps/desktop/src/components/chat/session.tsx(2 hunks)apps/desktop/src/components/settings/general/main-language.tsx(2 hunks)apps/desktop/src/store/zustand/ai-task/task-configs/enhance-transform.ts(1 hunks)apps/desktop/src/store/zustand/ai-task/task-configs/enhance-workflow.ts(1 hunks)apps/desktop/src/store/zustand/ai-task/task-configs/index.ts(2 hunks)apps/desktop/src/store/zustand/ai-task/task-configs/title-transform.ts(1 hunks)apps/desktop/src/store/zustand/ai-task/task-configs/title-workflow.ts(1 hunks)crates/template/assets/_language.partial.jinja(1 hunks)crates/template/assets/chat.system.jinja(1 hunks)crates/template/assets/enhance.system.jinja(1 hunks)crates/template/assets/title.system.jinja(1 hunks)crates/template/src/lib.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/src/store/zustand/ai-task/task-configs/index.tsapps/desktop/src/store/zustand/ai-task/task-configs/enhance-workflow.tsapps/desktop/src/chat/transport.tsapps/desktop/src/store/zustand/ai-task/task-configs/title-transform.tsapps/desktop/src/store/zustand/ai-task/task-configs/enhance-transform.tsapps/desktop/src/store/zustand/ai-task/task-configs/title-workflow.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/store/zustand/ai-task/task-configs/index.tsapps/desktop/src/store/zustand/ai-task/task-configs/enhance-workflow.tsapps/desktop/src/chat/transport.tsapps/desktop/src/store/zustand/ai-task/task-configs/title-transform.tsapps/desktop/src/store/zustand/ai-task/task-configs/enhance-transform.tsapps/desktop/src/components/chat/session.tsxapps/desktop/src/components/settings/general/main-language.tsxapps/desktop/src/store/zustand/ai-task/task-configs/title-workflow.ts
🧬 Code graph analysis (4)
apps/desktop/src/store/zustand/ai-task/task-configs/enhance-transform.ts (1)
crates/whisper-local/src/model/mod.rs (1)
language(26-28)
apps/desktop/src/components/chat/session.tsx (2)
crates/whisper-local/src/model/mod.rs (1)
language(26-28)apps/desktop/src/chat/transport.ts (1)
CustomChatTransport(12-60)
apps/desktop/src/components/settings/general/main-language.tsx (1)
extensions/shared/types/hypr-extension.d.ts (5)
Select(397-397)SelectTrigger(409-409)SelectValue(410-410)SelectContent(398-398)SelectItem(400-400)
apps/desktop/src/store/zustand/ai-task/task-configs/title-workflow.ts (1)
apps/desktop/src/store/zustand/ai-task/task-configs/index.ts (1)
TaskArgsMapTransformed(20-56)
⏰ 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). (7)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: desktop_ci (macos, depot-macos-14)
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
🔇 Additional comments (12)
apps/desktop/src/components/settings/general/main-language.tsx (1)
30-41: LGTM!The Select implementation correctly binds value/onChange and dynamically generates options from
supportedLanguages. The key usage withcodeis appropriate for unique ISO 639-1 codes.apps/desktop/src/chat/transport.ts (2)
13-17: Constructor extension for systemPrompt looks goodAdding the optional
systemPromptparameter and storing it on the instance is straightforward and keeps existing two‑argument call sites valid. No issues here.
24-27: Wiring systemPrompt into Agent.system matches the PR intentForwarding the stored
systemPromptinto the Agent’ssystemfield is the right place to inject the template‑driven system prompt; behavior should remain unchanged whensystemPromptisundefined.If you haven’t already, please double‑check the
Experimental_Agentoptions in theaiSDK docs to confirmsystemis nullable/optional and behaves as expected withundefined.apps/desktop/src/components/chat/session.tsx (1)
3-5: New imports align with usageBringing in
useStateandtemplateCommandscleanly matches the newuseTransportlogic below; import structure stays consistent with the rest of the file.If you’re upgrading or pinning
@hypr/plugin-template, please confirm thatcommands.renderis still the recommended API name in the current version.apps/desktop/src/store/zustand/ai-task/task-configs/enhance-workflow.ts (1)
70-74: Language-aware system prompt wiring for enhance looks correct
getSystemPromptnow threadsargs.languageintoenhance.system, matchingTaskArgsMapTransformed["enhance"]and the new language-enforcing template; no issues spotted.crates/template/src/lib.rs (1)
77-78: Embedded_language.partialregistration matches existing template patternDefining
LANGUAGE_PARTIAL_TPLand registering it as"_language.partial"in the non‑debug environment mirrors how other templates are embedded, so the new partial will be available in release builds as expected.Also applies to: 99-135
apps/desktop/src/store/zustand/ai-task/task-configs/title-workflow.ts (1)
55-65: Title system prompt now correctly uses task languageUsing
args.languagewhen renderingtitle.system(and returningresult.data) aligns the workflow with the new language-aware templates; the change is consistent with the transformed args shape.apps/desktop/src/store/zustand/ai-task/task-configs/index.ts (1)
20-55: Transformed task args now properly carrylanguagefor enhance and titleAdding
language: stringto bothenhanceandtitleinTaskArgsMapTransformedmatches the updated transform/workflow logic and supports the language-aware templates; the type surface looks coherent.crates/template/assets/enhance.system.jinja (1)
1-7: Language enforcement hook in enhance.system aligns with workflow changesUsing
enforce_language(language, "IMPORTANT: Generate all content in")matches the newgetSystemPromptpayload (which now includeslanguage) and cleanly centralizes language instructions in the shared partial.apps/desktop/src/store/zustand/ai-task/task-configs/enhance-transform.ts (1)
49-64: Enhance transform now reliably provides language for downstream templatesInjecting
language = getLanguage(store)into the transformed enhance args, withai_languagelookup and"en"fallback, ensuresenhance.systemalways receives a usable language value; the implementation is straightforward and consistent with the title transform.Also applies to: 71-74
crates/template/assets/chat.system.jinja (1)
1-7: No action required—languageis already being passed to the template context.The single
chat.systemrender call site inapps/desktop/src/components/chat/session.tsxalready passeslanguageexplicitly:.render("chat.system", { language }). The language value is obtained with a fallback default of"en", ensuring it is never undefined.crates/template/assets/title.system.jinja (1)
1-4: Thelanguagevariable is correctly passed to the template context in the rendering call atapps/desktop/src/store/zustand/ai-task/task-configs/title-workflow.ts:56-57, wheretemplateCommands.render("title.system", { language: args.language })provides the required parameter. No action needed.
1 participant
No description provided.