-
Notifications
You must be signed in to change notification settings - Fork 625
feat: add support for zero temperature #920
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
WalkthroughDefaulting for temperature is changed from falsy checks (||) to nullish coalescing (??) across multiple providers, server utilities, persistence, thread creation, and UI. This preserves explicit 0 values and only applies defaults when temperature is undefined or null. The UI also allows temperature 0.0 and preserves user-set values on config load. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 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 |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (2)
418-486: Stream lacks standardized stop/usage events.coreStream returns without emitting a final stop (and no usage); consumers expecting the standardized event sequence may hang or miss metrics. Add a stop event at end (and usage if available).
Apply:
@@ - try { + try { while (true) { @@ - reader.releaseLock() + reader.releaseLock() } + // Emit final stop to conform to standardized interface + // If you later parse usage, emit createStreamEvent.usage(...) before this. + // import { createStreamEvent } from '@shared/types/core/llm-events' at top. + yield createStreamEvent.stop('complete')And add import:
- import { BaseLLMProvider, SUMMARY_TITLES_PROMPT } from '../baseProvider' + import { BaseLLMProvider, SUMMARY_TITLES_PROMPT } from '../baseProvider' + import { createStreamEvent } from '@shared/types/core/llm-events'
118-126: Do not log secrets; mask tokens and API keys.Authorization preview and token substrings are sensitive. Remove/mask to meet “avoid logging sensitive information.”
- console.log( - ' Authorization:', - headers.Authorization ? `Bearer ${this.provider.apiKey?.substring(0, 10)}...` : 'NOT SET' - ) + console.log(' Authorization: [REDACTED]') @@ - console.log('📊 [GitHub Copilot] Token response data:') - console.log(` Token: ${data.token ? data.token.substring(0, 20) + '...' : 'NOT PRESENT'}`) + console.log('📊 [GitHub Copilot] Token response data:') + console.log(' Token: [REDACTED]') @@ - console.log(` API Key preview: ${this.provider.apiKey.substring(0, 20)}...`) + console.log(' API Key: [REDACTED]')Also applies to: 197-203, 645-646
🧹 Nitpick comments (9)
src/renderer/src/components/ChatConfig.vue (1)
439-439: Zero-temperature UI: LGTM.Allowing min 0 on the slider correctly preserves explicit 0.
Consider formatting the inline value with one decimal to avoid float artifacts:
- <span class="text-xs text-muted-foreground">{{ temperatureValue[0] }}</span> + <span class="text-xs text-muted-foreground">{{ Number(temperatureValue[0]).toFixed(1) }}</span>src/main/presenter/sqlitePresenter/tables/conversations.ts (1)
162-162: Preserve 0 in DB defaults: LGTM.Switching to
settings.temperature ?? 0.7correctly keeps explicit zero.Default drift exists across the codebase (0.6 vs 0.7). Recommend centralizing a single DEFAULT_TEMPERATURE constant in shared config and using it here.
src/main/presenter/llmProviderPresenter/providers/geminiProvider.ts (1)
652-652: Nullish default protects 0: LGTM.
temperature ?? 0.7is correct for completions.For consistency, apply the same coalescing in summaries/generateText/suggestions or move the fallback into
getGenerateContentConfigto avoid drift if callers passundefined.src/renderer/src/components/TitleView.vue (1)
166-169: Do-not-overwrite logic: LGTM; unify default.Only setting temperature when unset is correct. The fallback uses 0.6; elsewhere 0.7 is used.
Introduce a shared DEFAULT_TEMPERATURE (pick one value) and reference it here to eliminate inconsistency.
src/main/presenter/mcpPresenter/inMemoryServers/imageServer.ts (2)
127-129: Respect explicit 0: LGTM.
modelConfig?.temperature ?? 0.6preserves zero as intended.Align the fallback with a project-wide DEFAULT_TEMPERATURE to avoid mixed 0.6/0.7 defaults.
174-176: OCR path matches behavior: LGTM.Same preservation of zero on OCR; consistent with the query path.
Same note on centralizing DEFAULT_TEMPERATURE.
src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts (1)
659-666: Streaming payload preserves 0: LGTM.
temperature: temperature ?? 0.7is correct for the stream path.Apply the same nullish default in the non-streaming
completions()payload to keep behavior consistent across paths.src/main/presenter/llmProviderPresenter/providers/anthropicProvider.ts (1)
784-786: Approve: nullish coalescing is applied consistently; 0.0 preserved.Verified occurrences in src/main/presenter/llmProviderPresenter/providers/anthropicProvider.ts — lines 784, 797, 901, 912, 970, 981, 1045, 1269.
Optional: clamp temperature to provider-allowed range before sending (enforce MIN..MAX).
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts (1)
161-163: Approve: Nullish defaults applied in all Ollama calls. Recommend clampingtemperatureto Ollama’s accepted range (e.g. 0–1) to guard against potential 400 errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/presenter/llmProviderPresenter/providers/anthropicProvider.ts(8 hunks)src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/geminiProvider.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts(2 hunks)src/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts(5 hunks)src/main/presenter/mcpPresenter/inMemoryServers/imageServer.ts(2 hunks)src/main/presenter/sqlitePresenter/tables/conversations.ts(1 hunks)src/main/presenter/threadPresenter/index.ts(1 hunks)src/renderer/src/components/ChatConfig.vue(1 hunks)src/renderer/src/components/TitleView.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/threadPresenter/index.tssrc/renderer/src/components/TitleView.vuesrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/renderer/src/components/ChatConfig.vuesrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/main/presenter/threadPresenter/index.tssrc/renderer/src/components/TitleView.vuesrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/renderer/src/components/ChatConfig.vuesrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
src/main/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all Electron main-process code under src/main/
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
src/main/presenter/**
📄 CodeRabbit inference engine (AGENTS.md)
src/main/presenter/**: Organize main-process presenters under src/main/presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider)
Follow the Presenter pattern for main-process modules
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/main/presenter/threadPresenter/index.tssrc/renderer/src/components/TitleView.vuesrc/main/presenter/sqlitePresenter/tables/conversations.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/renderer/src/components/ChatConfig.vuesrc/main/presenter/mcpPresenter/inMemoryServers/imageServer.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
src/renderer/src/**/*
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
Use scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/{src,shell,floating}/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/{src,shell,floating}/**/*.vue: Use Vue 3 Composition API for all components
All user-facing strings must use i18n keys via vue-i18n (no hard-coded UI strings)
Use Tailwind CSS utilities and ensure styles are scoped in Vue components
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/src/components/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Organize UI components by feature within src/renderer/src/
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g., ChatInput.vue)
Files:
src/renderer/src/components/TitleView.vuesrc/renderer/src/components/ChatConfig.vue
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
src/main/presenter/llmProviderPresenter/providers/*.ts: Each file insrc/main/presenter/llmProviderPresenter/providers/*.tsshould handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Provider implementations must use acoreStreammethod that yields standardized stream events to decouple the main loop from provider-specific details.
ThecoreStreammethod in each Provider must perform a single streaming API request per conversation round and must not contain multi-round tool call loop logic.
Provider files should implement helper methods such asformatMessages,convertToProviderTools,parseFunctionCalls, andprepareFunctionCallPromptas needed for provider-specific logic.
All provider implementations must parse provider-specific data chunks and yield standardized events for text, reasoning, tool calls, usage, errors, stop reasons, and image data.
When a provider does not support native function calling, it must prepare messages using prompt wrapping (e.g.,prepareFunctionCallPrompt) before making the API call.
When a provider supports native function calling, MCP tools must be converted to the provider's format (e.g., usingconvertToProviderTools) and included in the API request.
Provider implementations should aggregate and yield usage events as part of the standardized stream.
Provider implementations should yield image data events in the standardized format when applicable.
Provider implementations should yield reasoning events in the standardized format when applicable.
Provider implementations should yield tool call events (tool_call_start,tool_call_chunk,tool_call_end) in the standardized format.
Provider implementations should yield stop events with appropriatestop_reasonin the standardized format.
Provider implementations should yield error events in the standardized format...
Files:
src/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts
src/main/presenter/mcpPresenter/inMemoryServers/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement new MCP tools under src/main/presenter/mcpPresenter/inMemoryServers/
Files:
src/main/presenter/mcpPresenter/inMemoryServers/imageServer.ts
⏰ 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). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (2)
src/main/presenter/threadPresenter/index.ts (1)
1050-1051: Creation respects 0 temperature: LGTM.Explicit 0 now flows into merged settings correctly.
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (1)
353-354: Nullish coalescing preserves explicit zero — approved.
Switching totemperature ?? 0.7correctly allows 0.0. No activetemperature ||usages remain; only a commented instance at src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts:654.
Summary by CodeRabbit