feat: Add xAI provider, upgrade Gemini SDK#494
Conversation
- Added GPT-5.1 to default chat models with pricing ($1.25 input, $10 output per million tokens) - Updated recommended models to include GPT-5.1 instead of GPT-4.1 - Introduced migration logic to transition settings from version 12 to 13 - Migration adds GPT-5.1 to user settings while preserving existing GPT-5 as custom model - Removed GPT-5 from default models (users keep it as custom model) - Added comprehensive tests to verify migration functionality
📝 WalkthroughWalkthroughUpdates default models, provider/type mappings, and pricing; adds migrations (12→13, 13→14) and tests; propagates providerMetadata and DeepSeek reasoning through request/response flows; ports Gemini to Changes
Sequence Diagram(s)sequenceDiagram
participant App as App / CLI
participant Migrator as SettingsMigrator
participant Defaults as DEFAULT_CHAT_MODELS_V13/V14
participant Merger as getMigratedChatModels
participant Store as SettingsStorage
App->>Migrator: detect settings version (12 or 13)
Migrator->>Defaults: load DEFAULT_CHAT_MODELS_V13/V14 & DEFAULT_PROVIDERS_V14
Migrator->>Merger: merge existing chatModels/providers with defaults
Merger-->>Migrator: merged chatModels/providers (preserve customs)
Migrator->>Store: set version = 13/14 and persist migrated settings
Store-->>App: return persisted migrated settings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
- Removed gpt-4.1, gpt-4.1-mini, and gpt-4.1-nano from default chat models - Updated migration v12 to v13 to handle removal of GPT-4.1 models - Added test to verify GPT-4.1 models are preserved as custom models during migration - Users with existing GPT-4.1 models will keep them as custom models
![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 (1)
src/settings/schema/migrations/12_to_13.ts (1)
24-205: DEFAULT_CHAT_MODELS_V13 snapshot is consistent but duplicates constants
DEFAULT_CHAT_MODELS_V13mirrors theDEFAULT_CHAT_MODELSlist insrc/constants.ts(same ids/models/providerType/providerId and nested options), which is good for this migration but introduces duplication that can drift if one list is changed without the other.If you want to reduce future maintenance risk, consider deriving one from the other (or sharing a common source) while still keeping the “v13 snapshot” semantics clear, e.g., by exporting a v13-specific array from the constants module and reusing it here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/constants.ts(3 hunks)src/settings/schema/migrations/12_to_13.test.ts(1 hunks)src/settings/schema/migrations/12_to_13.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/settings/schema/migrations/12_to_13.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/settings/schema/migrations/12_to_13.ts (2)
src/settings/schema/setting.types.ts (1)
SettingMigration(93-97)src/settings/schema/migrations/migrationUtils.ts (1)
getMigratedChatModels(56-90)
🔇 Additional comments (4)
src/constants.ts (3)
15-15: Chat recommendations now include gpt-5.1 – consistent with defaults
RECOMMENDED_MODELS_FOR_CHATnow referencesgpt-5.1, which is present inDEFAULT_CHAT_MODELSand priced inOPENAI_PRICES, so the recommendation list stays in sync with available/priceable models.
246-250: OpenAI default chat model switched to gpt-5.1 – aligns with migration intentThe OpenAI default chat model entry now uses
id/modelofgpt-5.1, andgpt-5is no longer in the default list, matching the stated behavior that gpt-5 becomes a custom model while gpt-5.1 is the new default.
446-461: Pricing entry for gpt-5.1 added and matches described ratesThe
OPENAI_PRICESmap now includesgpt-5.1with{ input: 1.25, output: 10 }, consistent with the PR description and with the existinggpt-5entry for users who keep that as a custom model.src/settings/schema/migrations/12_to_13.ts (1)
5-22: Migration behavior matches the described v12→v13 transitionThe migration:
- Bumps
versionto 13.- Rebuilds
chatModelsviagetMigratedChatModelsusingDEFAULT_CHAT_MODELS_V13.Given
getMigratedChatModelskeeps any models not in the new defaults as “custom” entries, this correctly:
- Adds
gpt-5.1as a default.- Removes
gpt-5and thegpt-4.1*family from the default set while preserving any existing instances in user settings as custom models.This lines up well with the PR description and the inline comment.
- Introduced a new utility function to check if a model supports predicted outputs. - Updated the applyChangesToFile function to conditionally include prediction logic for supported models: gpt-4o, gpt-4o-mini, gpt-4.1, gpt-4.1-mini, and gpt-4.1-nano.
… comment - Changed the default apply model ID to 'gpt-4.1-mini' and added a comment explaining its preference over gpt-5-mini due to performance considerations regarding predicted outputs.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/constants.tssrc/utils/chat/apply.ts
🔇 Additional comments (4)
src/utils/chat/apply.ts (2)
18-30: LGTM! Predicted outputs support check is well-implemented.The
startsWithapproach correctly handles versioned model IDs (e.g.,gpt-4o-2024-01-01). The exclusion of GPT-5 models from this list aligns with the comment inconstants.tsexplaining that GPT-5 models don't support predicted outputs.
159-173: Incorrect API structure — prediction.content should be a string, not an array of objects.The OpenAI Predicted Outputs API specifies that
prediction.contentmust be a string (the full predicted text), but the code currently passes an array of content block objects[{ type: 'text', text: ... }]. This structure appears to conflate the message content format with the predicted outputs format and will not work with the OpenAI API. Refactor to passcurrentFileContentandblockToApplyconcatenated as a single string value:prediction: { type: 'content', content: currentFileContent + blockToApply, },⛔ Skipped due to learnings
Learnt from: kevin-on Repo: glowingjade/obsidian-smart-composer PR: 372 File: src/core/llm/anthropic.ts:352-359 Timestamp: 2025-04-27T20:59:14.376Z Learning: In AnthropicProvider's parseRequestMessage method, the content property for assistant messages is already guaranteed to be a string by the type system, so additional type checking with `typeof content === 'string'` is unnecessary.src/constants.ts (2)
12-17: LGTM! Clear documentation of model selection rationale.The comment clearly explains why
gpt-4.1-miniis preferred overgpt-5-minifor apply tasks (predicted outputs support). The update toRECOMMENDED_MODELS_FOR_CHATto includegpt-5.1aligns with the PR objectives.
449-449: LGTM! Pricing entry added correctly.The pricing for
gpt-5.1matches the PR description ($1.25 per million input tokens, $10 per million output tokens).
- Changed the recommended chat model from 'gpt-5.1' to 'gpt-5.2'. - Updated the pricing for 'gpt-5.2' in the OPENAI_PRICES record. - Incremented the SETTINGS_SCHEMA_VERSION to 14 and added migration logic for version 13 to 14.
- Removed outdated models 'gpt-5-nano', 'gpt-4o', and 'gpt-4o-mini' from default chat models. - Added 'gpt-4.1-mini' to the default chat models and migration logic. - Updated the test cases to reflect changes in model IDs and ensure proper migration from version 13 to 14. - Adjusted pricing for 'gpt-5.2' in the OPENAI_PRICES record.
…ion tests - Updated the default chat model from 'claude-opus-4.1' to 'claude-opus-4.5' in constants and migration logic. - Added tests to ensure 'claude-opus-4.5' is correctly added as a new default model during migration from version 13 to 14. - Preserved 'claude-opus-4.1' as a custom model in migration tests.
- Added 'gemini-3-pro-preview' and 'gemini-3-flash-preview' as new default chat models in the migration logic. - Updated the constants to reflect the new model IDs and removed outdated models. - Enhanced migration tests to ensure the new models are correctly added and existing models are preserved during migration.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/constants.ts:
- Around line 247-252: DEFAULT_CHAT_MODELS is missing an entry for gpt-5.1; add
an object to DEFAULT_CHAT_MODELS with providerType: 'openai', providerId:
PROVIDER_TYPES_INFO.openai.defaultProviderId, id: 'gpt-5.1', and model:
'gpt-5.1' so the default models list matches the pricing entry for gpt-5.1;
ensure the new entry follows the same shape and ordering as the existing gpt-5.2
entry and run tests/linters.
- Around line 229-234: The Anthropic entry uses an invalid model identifier: the
object with providerType 'anthropic' and id 'claude-opus-4.5' should have its
model field replaced with a valid Claude Opus identifier; update the model
property (currently 'claude-opus-4-5') to one of the supported values such as
'claude-opus-4-20250514', 'claude-opus-4-1-20250805', or an alias like
'claude-opus-4-0'/'claude-opus-4-1' so the entry
(providerType/providerId/id/model) matches Anthropic’s documented model names.
In @src/settings/schema/migrations/13_to_14.ts:
- Around line 134-169: Update the incorrect Perplexity model identifiers so the
`model` field matches each provider entry's `id`: set the entry with id
'sonar-pro' to model 'sonar-pro', set 'sonar-reasoning-pro' to model
'sonar-reasoning-pro', and set 'sonar-reasoning' to model 'sonar-reasoning'
(leave 'sonar-deep-research' as-is). Locate these objects in the migration list
where providerId is 'perplexity' and replace the wrong 'sonar' values with the
matching model IDs to ensure API requests use the correct model identifiers.
🧹 Nitpick comments (1)
src/settings/schema/migrations/13_to_14.test.ts (1)
1-291: Good test coverage for core migration scenarios.The tests comprehensively cover version increment, model merging, and custom model preservation.
Consider adding tests for models with nested configuration objects (e.g.,
o4-miniwithreasoning, Perplexity models withweb_search_options) to verify the shallow merge behavior documented ingetMigratedChatModels. Per the FIXME inmigrationUtils.ts(lines 73-76), nested objects are overwritten rather than merged, which could affect users who customized these settings.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/constants.tssrc/settings/schema/migrations/13_to_14.test.tssrc/settings/schema/migrations/13_to_14.tssrc/settings/schema/migrations/index.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/settings/schema/migrations/13_to_14.test.ts (1)
src/settings/schema/migrations/13_to_14.ts (2)
migrateFrom13To14(26-33)DEFAULT_CHAT_MODELS_V14(54-176)
src/settings/schema/migrations/13_to_14.ts (2)
src/settings/schema/setting.types.ts (1)
SettingMigration(93-97)src/settings/schema/migrations/migrationUtils.ts (1)
getMigratedChatModels(56-90)
src/settings/schema/migrations/index.ts (2)
src/settings/schema/migrations/12_to_13.ts (1)
migrateFrom12To13(15-22)src/settings/schema/migrations/13_to_14.ts (1)
migrateFrom13To14(26-33)
🔇 Additional comments (7)
src/settings/schema/migrations/index.ts (1)
6-7: LGTM!The migration registry is correctly updated:
- Imports follow existing alphabetical ordering convention
- Version bump to 14 is consistent with the two new migrations
- Migration chain (12→13→14) is properly linked with no gaps
Also applies to: 18-18, 81-90
src/settings/schema/migrations/13_to_14.ts (2)
26-33: LGTM!The migration function follows the established pattern correctly, using
getMigratedChatModelsfor consistent model merging behavior.
5-25: Clear migration documentation.The comment effectively documents the intent of this migration, making it easy for future maintainers to understand what changed between schema versions.
src/constants.ts (4)
12-17: Clarify: PR description says GPT-5.1, but code adds GPT-5.2.The PR objectives state "Adds GPT-5.1 to default chat models" and "Updates recommended models to include GPT-5.1 instead of GPT-4.1", but the code adds
gpt-5.2toRECOMMENDED_MODELS_FOR_CHAT. Please confirm whether this is intentional or if the PR description needs updating.The rationale comment for
DEFAULT_APPLY_MODEL_IDis helpful and clear.
408-411: LGTM!Pricing entries for the new models are added correctly. Note that
gpt-5.1andgpt-5share identical pricing ($1.25/$10), which appears intentional based on the PR description.
426-428: Claude Opus 4.5 pricing confirmed accurate.The pricing for
claude-opus-4-5($5/$25 per million tokens) matches Anthropic's current published rates as of January 2026. The substantial difference fromclaude-opus-4-1($15/$75) is correct.
276-286: Verifygemini-3-flash-previewmodel identifier.
gemini-3-pro-previewappears to be a valid official Gemini API identifier, butgemini-3-flash-previewis not listed in the current official Gemini model catalog. Confirm this identifier matches an actual available Gemini API model name before deployment.
- Replaced the deprecated '@google/generative-ai' package with '@google/genai' to support the latest models and features. - Updated the GeminiProvider class to utilize the new API structure and methods from '@google/genai'. - Enhanced chat message serialization and deserialization to include provider metadata. - Added support for thought signatures in function calls to improve response handling. - Updated related types and utility functions to accommodate changes in the new API.
- Added comprehensive settings for the Gemini model, including thinking level, budget, and thought summaries. - Implemented a new ThinkingConfig structure to manage thinking settings based on model type. - Updated the GeminiProvider to extract and include reasoning in responses. - Enhanced the AddChatModelModal to support the new provider settings for Gemini.
- Introduced the xAI provider with necessary configurations and API integration. - Added new chat models 'grok-4-1-fast' and 'grok-4-1-fast-non-reasoning' to the default settings. - Updated migration logic to include the xAI provider and new models. - Enhanced pricing calculations to support xAI model costs. - Updated schemas to accommodate the new xAI provider type.
- Updated DeepSeekMessageAdapter to include reasoning content in both non-streaming and streaming responses. - Introduced providerMetadata to store reasoning content for assistant messages during tool call iterations. - Refactored request and response types to accommodate new RequestProviderMetadata structure for DeepSeek integration.
- Introduced the xAI provider and updated associated configurations. - Removed the Groq and Morph providers, converting their models to openai-compatible. - Updated migration logic to reflect these changes and ensure proper handling of existing models. - Enhanced schemas to accommodate the new provider types and updated chat model definitions.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/core/llm/deepseekMessageAdapter.ts (1)
54-82: Potential data loss for incrementally streamed reasoning content.In streaming mode,
reasoningContentis extracted per chunk and stored inproviderMetadata. However,responseGenerator.tsuses a "keep first" strategy forproviderMetadata, meaning only the first chunk'sreasoningContentis preserved.If DeepSeek streams
reasoning_contentacross multiple chunks, theproviderMetadata.deepseek.reasoningContentfield will be incomplete when sent back during tool call iterations, even though the accumulatedreasoningfield (for display) would be complete.Consider one of these approaches:
- Accumulate
reasoningContentinproviderMetadatasimilar to howreasoningis accumulated- Reconstruct
providerMetadata.deepseek.reasoningContentfrom the final accumulatedreasoningbefore making tool call requests- Confirm that DeepSeek's API sends
reasoning_contentin a single chunksrc/utils/chat/responseGenerator.ts (1)
282-302: Fix providerMetadata accumulation for DeepSeek's incrementally streamed reasoning_content.DeepSeek sends
reasoning_contentincrementally across multiple streaming chunks, not once. The current implementation keeps only the firstproviderMetadatareceived (line 301-302), which discards subsequentreasoning_contentchunks. While thereasoningfield properly accumulates across chunks (lines 290-292),providerMetadata.deepseek.reasoningContentis lost.Update the logic to accumulate
providerMetadatasimilarly to howreasoningis concatenated, or ensureproviderMetadata.deepseek.reasoningContentis merged across chunks rather than discarded.src/settings/schema/migrations/1_to_2.ts (1)
691-702: Fix ESLint pipeline failure: wrapollamaApplyProviderIdwithString().The pipeline is failing because
ollamaApplyProviderIdhas typeany(derived from the V2LLMProvider type alias), which violates@typescript-eslint/restrict-template-expressions.🔧 Proposed fix
let ollamaApplyModelId if (existingSameChatModelForApplyModel) { ollamaApplyModelId = existingSameChatModelForApplyModel.id } else { - ollamaApplyModelId = `${ollamaApplyProviderId}/${data.ollamaApplyModel.model}` + ollamaApplyModelId = `${String(ollamaApplyProviderId)}/${data.ollamaApplyModel.model}` chatModels.push({
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 50: Update the invalid dependency version for "@google/genai" in
package.json from "^1.35.0" to the published "^1.34.0"; after updating the
version string, run your package manager (npm/yarn/pnpm) to reinstall and
regenerate the lockfile so the dependency graph is consistent and CI installs
succeed.
In `@src/constants.ts`:
- Around line 288-298: Update the incorrect Grok model identifier in the
constants list: change the object with id 'grok-4-1-fast' (where providerType is
'xai' and providerId uses PROVIDER_TYPES_INFO.xai.defaultProviderId) so its
model and id use 'grok-4-1-fast-reasoning' instead of 'grok-4-1-fast' to match
xAI's API; ensure the other entry 'grok-4-1-fast-non-reasoning' remains
unchanged.
♻️ Duplicate comments (2)
src/constants.ts (2)
220-221: Verifyclaude-opus-4-5model identifier with Anthropic API.This was flagged in a previous review. The model identifier
claude-opus-4-5should be verified against Anthropic's current API documentation to ensure it's a valid model name.Anthropic claude-opus-4-5 model identifier API 2025
238-239: PR description inconsistency: GPT-5.1 not in default models.The PR description states "Adds GPT-5.1 to default chat models" but only
gpt-5.2is present here. The pricing forgpt-5.1exists at line 359, but there's no corresponding entry inDEFAULT_CHAT_MODELS. Either add thegpt-5.1model entry or update the PR description.
🧹 Nitpick comments (7)
src/settings/schema/migrations/2_to_3.ts (1)
48-91: Shallow copy causes mutation of original data.The spread
{ ...data }on line 49 creates a shallow copy, sonewData.providersandnewData.chatModelsstill reference the same arrays (and objects) asdata. The in-place mutations on lines 63 and 82 modify the original input, which could cause unexpected side effects if callers rely ondatabeing unchanged.♻️ Suggested fix: deep-copy the arrays before mutation
export const migrateFrom2To3: SettingMigration['migrate'] = (data) => { const newData = { ...data } newData.version = 3 // Handle providers migration if ('providers' in newData && Array.isArray(newData.providers)) { + // Deep copy to avoid mutating original data + newData.providers = newData.providers.map((p) => ({ ...p })) const existingProvidersMap = new Map( newData.providers.map((provider) => [provider.id, provider]), ) // Add new providers, overriding provider type if ID exists for (const newProvider of NEW_DEFAULT_PROVIDERS) { const existingProvider = existingProvidersMap.get(newProvider.id) if (existingProvider) { // Override the provider type while keeping other settings existingProvider.type = newProvider.type } else { // Add new provider newData.providers.push({ ...newProvider }) } } } // Handle chat models migration if ('chatModels' in newData && Array.isArray(newData.chatModels)) { + // Deep copy to avoid mutating original data + newData.chatModels = newData.chatModels.map((m) => ({ ...m })) const existingModelsMap = new Map( newData.chatModels.map((model) => [model.id, model]), )src/types/llm/request.ts (1)
68-75: Consider unifying withResponseProviderMetadata.This type has an identical structure to
ResponseProviderMetadatainsrc/types/llm/response.ts. You could define a singleProviderMetadatatype and re-export or alias it in both modules to reduce duplication.src/settings/schema/migrations/1_to_2.ts (2)
5-11: Migration-localanytypes are pragmatic but cause ESLint issues downstream.Using
anyto freeze v2 state is a reasonable approach for migration stability. However, this causes ESLintrestrict-template-expressionserrors when these values are used in template literals (see pipeline failure at line 695).Consider adding explicit
String()coercion at template literal usage sites, or define minimal structural types instead ofany:// Minimal structural types that capture only what the migration needs type V2LLMProvider = { type: string; id: string; baseUrl?: string; apiKey?: string } type V2ChatModel = { providerType: string; providerId: string; id: string; model: string; streamingDisabled?: boolean } type V2EmbeddingModel = { providerType: string; providerId: string; id: string; model: string; dimension: number }
649-660: Consider addingString()coercion here for consistency.Similar to line 695, this template literal uses a provider ID that may be typed as
any. For consistency and to prevent future ESLint issues:♻️ Suggested change
if (data.ollamaChatModel.model) { - const ollamaChatModelId = `${ollamaChatProviderId}/${data.ollamaChatModel.model}` + const ollamaChatModelId = `${String(ollamaChatProviderId)}/${data.ollamaChatModel.model}` chatModels.push({src/components/settings/sections/models/ChatModelSettings.tsx (1)
257-263: Consider adding budget range validation.The budget validation only checks for
NaN. Per the UI description (line 347), valid values include-1for dynamic and0to disable. Consider adding validation to ensure the budget is either-1or a non-negative number to prevent invalid configurations.🔧 Suggested improvement
if (controlMode === 'budget') { parsedBudget = parseInt(thinkingBudget, 10) if (isNaN(parsedBudget)) { new Notice('Please enter a valid number for thinking budget') return } + if (parsedBudget < -1) { + new Notice('Thinking budget must be -1 (dynamic), 0 (disabled), or a positive number') + return + } }src/core/llm/gemini.ts (2)
228-272: Potential edge case: thoughtSignature lost when content is empty.If an assistant message has an empty
contentstring (''), no tool calls, but has athoughtSignature, the signature will be lost since no parts are created (line 235 skips empty content, line 270 returns null).This is likely a rare edge case, but consider whether the signature should be preserved in such scenarios or if this case is impossible in practice.
517-542: Consider enforcing mutual exclusivity between thinkingLevel and thinkingBudget.The
buildThinkingConfigmethod can set boththinkingLevelandthinkingBudgetif the model settings have both defined. Per the UI (ChatModelSettings.tsx), these are meant to be mutually exclusive based oncontrol_mode. However, if user settings somehow have both values, both would be sent to the API.Consider adding logic to only set one based on the model's intended use (Gemini 3 uses level, Gemini 2.5 uses budget).
🔧 Suggested improvement
private static buildThinkingConfig( model: ChatModel & { providerType: 'gemini' }, ): ThinkingConfig | undefined { if (!model.thinking?.enabled) { return undefined } const config: ThinkingConfig = {} - if (model.thinking.thinking_level) { + // Use control_mode to determine which setting to apply + // Level for Gemini 3, Budget for Gemini 2.5 + if (model.thinking.control_mode === 'level' && model.thinking.thinking_level) { const level = this.THINKING_LEVEL_MAP[model.thinking.thinking_level] if (level) { config.thinkingLevel = level } - } - - if (model.thinking.thinking_budget !== undefined) { + } else if (model.thinking.control_mode === 'budget' && model.thinking.thinking_budget !== undefined) { config.thinkingBudget = model.thinking.thinking_budget } if (model.thinking.include_thoughts) { config.includeThoughts = model.thinking.include_thoughts } return config }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (23)
package.jsonsrc/components/settings/modals/AddChatModelModal.tsxsrc/components/settings/sections/models/ChatModelSettings.tsxsrc/constants.tssrc/core/llm/deepseekMessageAdapter.tssrc/core/llm/gemini.tssrc/core/llm/manager.tssrc/core/llm/morphProvider.tssrc/core/llm/xaiProvider.tssrc/hooks/useChatHistory.tssrc/settings/schema/migrations/13_to_14.test.tssrc/settings/schema/migrations/13_to_14.tssrc/settings/schema/migrations/1_to_2.tssrc/settings/schema/migrations/2_to_3.tssrc/types/chat-model.types.tssrc/types/chat.tssrc/types/embedding-model.types.tssrc/types/llm/request.tssrc/types/llm/response.tssrc/types/provider.types.tssrc/utils/chat/promptGenerator.tssrc/utils/chat/responseGenerator.tssrc/utils/llm/price-calculator.ts
💤 Files with no reviewable changes (1)
- src/core/llm/morphProvider.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-04-27T19:00:27.177Z
Learnt from: kevin-on
Repo: glowingjade/obsidian-smart-composer PR: 372
File: src/types/chat.ts:101-110
Timestamp: 2025-04-27T19:00:27.177Z
Learning: Tool-call-related types were extracted into a dedicated module (src/types/tool-call.types.ts) to avoid circular dependencies between chat.ts and llm/request.ts, creating a single source of truth.
Applied to files:
src/types/provider.types.tssrc/core/llm/deepseekMessageAdapter.tssrc/utils/llm/price-calculator.tssrc/types/chat.tssrc/settings/schema/migrations/2_to_3.tssrc/core/llm/gemini.tssrc/types/llm/request.tssrc/settings/schema/migrations/1_to_2.tssrc/types/chat-model.types.ts
📚 Learning: 2025-04-27T20:59:14.376Z
Learnt from: kevin-on
Repo: glowingjade/obsidian-smart-composer PR: 372
File: src/core/llm/anthropic.ts:352-359
Timestamp: 2025-04-27T20:59:14.376Z
Learning: In AnthropicProvider's parseRequestMessage method, the content property for assistant messages is already guaranteed to be a string by the type system, so additional type checking with `typeof content === 'string'` is unnecessary.
Applied to files:
src/core/llm/deepseekMessageAdapter.tssrc/utils/chat/promptGenerator.tssrc/utils/chat/responseGenerator.tssrc/types/chat.tssrc/types/llm/request.tssrc/hooks/useChatHistory.ts
📚 Learning: 2025-04-27T18:49:51.611Z
Learnt from: kevin-on
Repo: glowingjade/obsidian-smart-composer PR: 372
File: src/core/llm/anthropic.ts:521-527
Timestamp: 2025-04-27T18:49:51.611Z
Learning: When handling Anthropic streaming responses, the `input_json_delta` chunks only contain argument updates without the function name field. The complete metadata (ID, type, name) is provided in the initial `content_block_start` event, and these two types of events should be merged properly during streaming response processing.
Applied to files:
src/utils/chat/responseGenerator.ts
🧬 Code graph analysis (11)
src/core/llm/deepseekMessageAdapter.ts (3)
src/core/llm/openaiMessageAdapter.ts (1)
OpenAIMessageAdapter(23-202)src/types/llm/response.ts (1)
LLMResponseNonStreaming(12-15)src/types/llm/request.ts (1)
RequestMessage(88-92)
src/utils/llm/price-calculator.ts (1)
src/constants.ts (1)
XAI_PRICES(390-393)
src/types/chat.ts (1)
src/types/llm/request.ts (1)
RequestProviderMetadata(68-75)
src/settings/schema/migrations/13_to_14.ts (2)
src/settings/schema/setting.types.ts (1)
SettingMigration(93-97)src/settings/schema/migrations/migrationUtils.ts (2)
getMigratedProviders(9-39)getMigratedChatModels(56-90)
src/core/llm/gemini.ts (3)
src/types/llm/response.ts (2)
LLMResponseStreaming(17-20)LLMResponseNonStreaming(12-15)src/types/llm/request.ts (1)
RequestTool(98-101)src/types/chat-model.types.ts (1)
ChatModel(109-109)
src/types/llm/request.ts (1)
src/types/tool-call.types.ts (1)
ToolCallRequest(1-5)
src/core/llm/xaiProvider.ts (1)
src/types/provider.types.ts (1)
LLMProvider(90-90)
src/settings/schema/migrations/13_to_14.test.ts (1)
src/settings/schema/migrations/13_to_14.ts (2)
migrateFrom13To14(76-86)DEFAULT_CHAT_MODELS_V14(117-200)
src/components/settings/modals/AddChatModelModal.tsx (1)
src/types/chat-model.types.ts (1)
ChatModel(109-109)
src/components/settings/sections/models/ChatModelSettings.tsx (1)
src/types/chat-model.types.ts (2)
ChatModel(109-109)chatModelSchema(28-107)
src/core/llm/manager.ts (1)
src/core/llm/xaiProvider.ts (1)
XaiProvider(18-65)
🪛 GitHub Actions: CI
src/settings/schema/migrations/1_to_2.ts
[error] 695-695: ESLint: Invalid type 'any' of template literal expression. @typescript-eslint/restrict-template-expressions
🪛 Gitleaks (8.30.0)
src/settings/schema/migrations/13_to_14.test.ts
[high] 151-151: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (38)
src/settings/schema/migrations/2_to_3.ts (1)
3-8: Good practice: hardcoded constants for migration stability.Decoupling migrations from current constants ensures this migration remains stable even if
PROVIDER_TYPES_INFOchanges in future versions.src/components/settings/modals/AddChatModelModal.tsx (1)
106-114: LGTM!The type cast is necessary when updating the discriminant field (
providerType) of a discriminated union. The runtime validation viachatModelSchema.safeParseon submit provides safety against invalid state.src/types/llm/request.ts (1)
77-82: LGTM!Adding
providerMetadatatoRequestAssistantMessageenables round-tripping provider-specific context (like Gemini's thought signatures and DeepSeek's reasoning content) through multi-turn conversations.src/types/chat.ts (3)
6-6: LGTM!Import correctly added to bring in
RequestProviderMetadataalongside other LLM types.
21-33: LGTM!Adding
providerMetadatatoChatAssistantMessageenables preserving provider-specific context (Gemini thought signatures, DeepSeek reasoning) across conversation turns.
63-75: LGTM!
SerializedChatAssistantMessageis correctly updated in parallel withChatAssistantMessage, ensuringproviderMetadatapersists through serialization.src/types/llm/response.ts (3)
30-37: LGTM!
ResponseProviderMetadatacleanly defines provider-specific metadata for responses. As noted inrequest.ts, this type is identical toRequestProviderMetadataand could potentially be unified.
39-50: LGTM!Non-streaming choice correctly extended with
providerMetadatato surface provider-specific data from the LLM response.
52-63: LGTM!Streaming choice delta correctly mirrors the non-streaming structure, ensuring consistent metadata propagation for both response modes.
src/hooks/useChatHistory.ts (2)
141-151: LGTM!The serialization of
providerMetadatafor assistant messages is consistent with the existing pattern for other optional fields likemetadata,reasoning, andannotations.
178-188: LGTM!The deserialization correctly restores
providerMetadatafrom the serialized form, maintaining symmetry with the serialization logic.src/utils/chat/promptGenerator.ts (1)
202-213: LGTM!Including
providerMetadatain the assistantRequestMessageenables provider-specific data (like DeepSeek's reasoning content) to be passed back during tool call iterations, which is essential for maintaining conversation context with providers that require it.src/core/llm/deepseekMessageAdapter.ts (2)
23-52: LGTM on non-streaming response parsing.The extraction of
reasoning_contentand its storage in bothreasoning(for display) andproviderMetadata(for API round-trip) is well-structured. The type cast is acceptable for accessing vendor-specific extensions not present in OpenAI's types.
84-100: LGTM on the request message parsing.The method correctly:
- Delegates to the parent class for base message construction
- Conditionally injects
reasoning_contentonly for assistant messages with the relevant metadata- Uses the type cast appropriately since
reasoning_contentis a DeepSeek-specific extensionsrc/types/provider.types.ts (1)
17-41: LGTM! Provider implementations verified.The xAI, deepseek, and perplexity provider additions are properly implemented across all required files:
- ✓
src/core/llm/manager.ts: XaiProvider imported and case handler present- ✓
src/constants.ts: xAI configuration with defaultProviderId defined- ✓
src/types/chat-model.types.ts: xAI literal included in schema- ✓
src/types/embedding-model.types.ts: xAI literal included in schemasrc/core/llm/xaiProvider.ts (2)
18-34: LGTM!The XaiProvider implementation correctly:
- Extracts the 'xai' provider type from the union
- Normalizes the baseURL by stripping trailing slashes (line 30)
- Defaults to the correct xAI API endpoint
- Uses the OpenAI SDK with the message adapter pattern consistent with other providers
60-64: LGTM!Good use of
String(this.provider.id)to safely coerce the provider ID in the template literal, avoiding potential ESLintrestrict-template-expressionsissues.src/types/chat-model.types.ts (2)
52-65: LGTM!The Gemini thinking configuration is well-structured:
control_modediscriminates between level-based (Gemini 3) and budget-based (Gemini 2.5) approaches- Comments clearly document the purpose of each field
- All fields are appropriately optional to support various model capabilities
66-107: LGTM!Provider type schema changes are consistent with the manager.ts implementation. The discriminated union covers all provider types handled in
getProviderClient.src/utils/llm/price-calculator.ts (2)
1-6: LGTM!Clean addition of XAI_PRICES import alongside existing provider price constants.
46-54: LGTM!The xAI pricing calculation follows the established pattern used by other providers. The null-return for unsupported models ensures graceful degradation.
src/core/llm/manager.ts (3)
18-18: LGTM!XaiProvider import added correctly.
65-67: LGTM!The 'xai' case correctly instantiates XaiProvider with the provider configuration. This aligns with the provider type definition in
chat-model.types.ts.
37-74: Switch statement covers all provider types.The switch statement handles all 12 provider types defined in the
chatModelSchemadiscriminated union, ensuring exhaustive coverage. TypeScript will enforce this at compile time.src/components/settings/sections/models/ChatModelSettings.tsx (1)
228-378: Well-structured Gemini thinking settings implementation.The settings component follows the established patterns from other providers and correctly implements conditional UI for level vs. budget control modes. The schema validation ensures type safety before persisting settings.
src/types/embedding-model.types.ts (1)
35-62: Provider types align with migration changes.The embedding model schema correctly reflects the updated provider types, including the new
xaiprovider and removal of legacy providers (groq,morph). This is consistent with the provider changes in the v13→v14 migration.src/settings/schema/migrations/13_to_14.ts (2)
56-66: Verify baseUrl preservation logic.Line 63 uses
p.baseUrl || LEGACY_PROVIDERS[p.type]which treats empty string''as falsy, falling back to the default URL. This is likely the intended behavior, but verify that an explicitly set empty baseUrl should fall back to the default rather than preserving the empty value.
117-200: Default models list is well-structured.The new default models cover major providers with appropriate configurations. The
o4-minimodel correctly includes reasoning settings withmediumeffort as default.src/settings/schema/migrations/13_to_14.test.ts (3)
428-458: Good documentation of shallow merge behavior.The test correctly documents that
Object.assigndoes shallow merging, so user's customreasoning_effort: 'high'gets overwritten by the default'medium'. The comment at line 453 explicitly notes this behavior, which is helpful for future maintainers.Note: The FIXME comment in
migrationUtils.ts(lines 17-20, 70-73) already tracks this as a known limitation. Consider addressing the deep merge TODO if preserving nested user settings becomes a priority.
679-717: Test data reflects historical model identifier issues.The Perplexity models in the test input (e.g.,
sonar-prowithmodel: 'sonar'at line 683) intentionally reflect incorrect historical configurations that users may have. This tests that the migration preserves existing user settings as custom models even when they contain incorrect identifiers, which is the correct behavior.
143-191: Static analysis false positive - test API keys.The Gitleaks warning about a "generic API key" at line 151 is a false positive. All API keys in this test file are obviously fake placeholder values (e.g.,
'mistral-key-789','sk-openai-secret-key','test-*-api-key'). No remediation needed.src/core/llm/gemini.ts (2)
552-556: Embedding API migration looks correct.The embedding retrieval correctly uses the new
client.models.embedContentAPI and safely handles the response with optional chaining and a fallback to an empty array.
1-12: API migration to@google/genaiis correctly implemented.The code properly implements the
@google/genaiSDK API:
- Client initialization at line 56:
new GoogleGenAI({ apiKey: ... })matches SDK requirements- Method calls at lines 82+ use
client.models.generateContent()per SDK specificationNo API compatibility issues found.
src/constants.ts (5)
12-14: Helpful documentation on model selection rationale.The comment clearly explains the technical reasoning for preferring
gpt-4.1-miniovergpt-5-minifor apply tasks. The model is correctly present inDEFAULT_CHAT_MODELS(lines 247-252).
17-17: LGTM!The recommended models list is updated consistently with the new
gpt-5.2entry inDEFAULT_CHAT_MODELS.
48-103: Provider type additions look correct.The new provider types follow the established pattern. Local inference providers (
ollama,lm-studio) appropriately haverequireApiKey: false, and the embedding support flags align with provider capabilities.
358-399: Pricing tables properly extended.The new pricing entries follow the established pattern. The version comment for DeepSeek (
DeepSeek-V3.2) is a good documentation practice that aids future maintenance.
266-274: The model identifiersgemini-3-pro-previewandgemini-3-flash-previeware correct and match Google's official Gemini 3 API documentation. These preview models are expected to evolve (similar togemini-2.5-pro's progression from preview to official release), and the codebase already has migration infrastructure to handle future changes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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 (3)
src/core/llm/gemini.ts (3)
227-278: Bug risk: assistantmessage.contentmay be null/undefined (still pushed as text)
if (message.content !== '')will treatnull/undefinedas “not empty string” and attempt to push{ text: message.content }, which can cause bad requests or runtime issues.Proposed fix (null-safe + avoids sending empty text parts)
- // Add text content part - if (message.content !== '') { + const text = + typeof message.content === 'string' ? message.content : '' + + // Add text content part + if (text.trim() !== '') { // If no tool calls and we have a signature, attach it to the text part if (!hasToolCalls && thoughtSignature) { - contentParts.push({ text: message.content, thoughtSignature }) + contentParts.push({ text, thoughtSignature }) } else { - contentParts.push({ text: message.content }) + contentParts.push({ text }) } }
335-372: Streaming: tool-call IDs may not be stable across chunks
If the SDK re-emits functionCalls across multiple chunks without anid,uuidv4()will generate a new id each chunk, which can break downstream “merge deltas” logic. Consider deriving a deterministic id whenf.idis absent (e.g.,${messageId}:${index}:${name}), or persist a map in the generator.Example deterministic id (stateless)
- private static parseFunctionCallsForStreaming( + private static parseFunctionCallsForStreaming( functionCalls: FunctionCall[] | undefined, + messageId?: string, ) { return functionCalls?.map((f, index) => ({ index, - id: f.id ?? uuidv4(), + id: f.id ?? `${messageId ?? 'msg'}:${index}:${f.name ?? 'function'}`, type: 'function' as const, function: { name: f.name ?? '', arguments: JSON.stringify(f.args ?? {}), }, })) }And pass
messageIdfromparseStreamingResponseChunk.
468-486: Tool schema: missingrequired(andpropertiesmay be undefined)
Iftool.function.parametershasrequired, dropping it can change tool-call behavior. AlsocleanedParameters.propertiesmay be missing, and the cast will explode later.Proposed fix (preserve required + safe default)
return { functionDeclarations: [ { name: tool.function.name, description: tool.function.description, parameters: { type: Type.OBJECT, - properties: cleanedParameters.properties as Record<string, Schema>, + properties: + (cleanedParameters.properties as Record<string, Schema> | undefined) ?? + {}, + required: cleanedParameters.required as string[] | undefined, }, }, ], }
🤖 Fix all issues with AI agents
In `@src/core/llm/gemini.ts`:
- Around line 505-542: In buildThinkingConfig (class-level THINKING_LEVEL_MAP
and method buildThinkingConfig), the check "if
(model.thinking.include_thoughts)" ignores explicit false; change the gating to
test for undefined (e.g., "if (model.thinking.include_thoughts !== undefined)")
so includeThoughts is set when the caller explicitly passes false or true; keep
the rest of the mapping for thinkingLevel/thinkingBudget unchanged.
- Around line 295-333: In parseNonStreamingResponse, stop mixing response.text
and response.functionCalls with candidate parts; instead treat
response.candidates?.[0]?.content?.parts as the canonical source: iterate the
parts array, concatenate text-type parts into the final content string and
collect functionCall-type parts into the functionCalls structure passed to
GeminiProvider.parseFunctionCalls (or adapt parseFunctionCalls to accept parts),
then remove usage of response.text and response.functionCalls and use the
extracted content and functionCalls when building the returned choice (preserve
existing use of extractThoughtSignature and extractThoughtSummaries on parts).
- Around line 81-101: The abortSignal (options?.signal) is incorrectly passed
inside the config object to this.client.models.generateContent and thus ignored;
move abortSignal out of config to the top-level request parameter when calling
this.client.models.generateContent (keep thinkingConfig inside config), e.g.,
remove abortSignal from the config and add abortSignal: options?.signal
alongside model, contents, and config; apply the same change to the other
generateContent call that uses GeminiProvider.parseRequestTool /
GeminiProvider.buildThinkingConfig so cancellations work correctly.
🧹 Nitpick comments (1)
src/core/llm/gemini.ts (1)
184-192: Streaming generator typing may be too narrow
If the SDK yields a different chunk type thanGenerateContentResponse(or wraps it), this signature will fight TypeScript and/or break at runtime. Consider typing to the SDK’s exported stream chunk type (or infer from the SDK call).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/llm/gemini.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: namyeop
Repo: glowingjade/obsidian-smart-composer PR: 433
File: src/settings/schema/migrations/10_to_11.test.ts:20-20
Timestamp: 2025-07-11T08:35:40.553Z
Learning: The Gemini 2.5 Pro model naming evolution: preview-03-25 → exp-03-25 (migration 8→9) → gemini-2.5-pro (migration 10→11). Migration 10→11 should migrate from exp-03-25, not preview-03-25.
📚 Learning: 2025-04-27T19:00:27.177Z
Learnt from: kevin-on
Repo: glowingjade/obsidian-smart-composer PR: 372
File: src/types/chat.ts:101-110
Timestamp: 2025-04-27T19:00:27.177Z
Learning: Tool-call-related types were extracted into a dedicated module (src/types/tool-call.types.ts) to avoid circular dependencies between chat.ts and llm/request.ts, creating a single source of truth.
Applied to files:
src/core/llm/gemini.ts
🔇 Additional comments (4)
src/core/llm/gemini.ts (4)
399-443: Thought extraction looks reasonable; confirmthought/thoughtSignaturefields exist onPart
Logic matches your comment (signature on first functionCall part else last part). Just verify the SDK actually populatespart.thoughtandpart.thoughtSignaturein v1.35.0 for the models you’re targeting.
551-557: The code is correct as written. The@google/genaiSDK (v1.35.0 per package.json) expects thecontentsparameter to accept a raw string, exactly as shown in the official documentation example. The SDK handles the conversion to the required structured format internally.Likely an incorrect or invalid review comment.
124-165: No action needed—implementation is correct for SDK v1.35.0The return type from
generateContentStream()in@google/genaiv1.35.0 is alreadyPromise<AsyncGenerator<GenerateContentResponse>>. Afterawait,streamis the async generator itself (not a wrapper object), which matches thestreamResponseGeneratorparameter type. Additionally,abortSignalandthinkingConfigare correctly placed in theconfigobject, matching the non-streaminggenerateContent()method exactly.Likely an incorrect or invalid review comment.
1-12: No changes needed —@google/genaiv1.35.0 usage is correctThe constructor
new GoogleGenAI({ apiKey })is correct for v1.35.0. Themodels.generateContentandmodels.generateContentStreamAPI shapes match the types imported (GenerateContentResponse, FunctionCall, etc.), andmodels.embedContentcorrectly accepts string/Content inputs. Obsidian's current Electron runtime (v39.x with Node 22.21.1) includescrypto.randomUUID()(available since Node v14.17), so no fallback is needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| try { | ||
| const model = this.client.getGenerativeModel({ | ||
| const result = await this.client.models.generateContent({ | ||
| model: request.model, | ||
| generationConfig: { | ||
| contents: request.messages | ||
| .map((message) => GeminiProvider.parseRequestMessage(message)) | ||
| .filter((m): m is Content => m !== null), | ||
| config: { | ||
| maxOutputTokens: request.max_tokens, | ||
| temperature: request.temperature, | ||
| topP: request.top_p, | ||
| presencePenalty: request.presence_penalty, | ||
| frequencyPenalty: request.frequency_penalty, | ||
| }, | ||
| systemInstruction: systemInstruction, | ||
| }) | ||
|
|
||
| const result = await model.generateContent( | ||
| { | ||
| systemInstruction: systemInstruction, | ||
| contents: request.messages | ||
| .map((message) => GeminiProvider.parseRequestMessage(message)) | ||
| .filter((m): m is Content => m !== null), | ||
| abortSignal: options?.signal, | ||
| tools: request.tools?.map((tool) => | ||
| GeminiProvider.parseRequestTool(tool), | ||
| ), | ||
| thinkingConfig: GeminiProvider.buildThinkingConfig(model), | ||
| }, | ||
| { | ||
| signal: options?.signal, | ||
| }, | ||
| ) | ||
| }) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file
find . -name "gemini.ts" -type fRepository: glowingjade/obsidian-smart-composer
Length of output: 103
🏁 Script executed:
# Let's also look at the package.json to confirm the SDK version
cat package.json | grep -A 5 -B 5 "@google"Repository: glowingjade/obsidian-smart-composer
Length of output: 436
🏁 Script executed:
# Now let's examine the specific file around the mentioned lines
head -150 src/core/llm/gemini.ts | tail -80Repository: glowingjade/obsidian-smart-composer
Length of output: 2778
🌐 Web query:
Google Genai SDK v1.35.0 generateContent API signature thinkingConfig abortSignal
💡 Result:
Short answer — in the @google/genai SDK (v1.35.0 era) generateContent accepts a config that can include thinking configuration, and you may pass an AbortSignal to cancel the request.
Example shape (JS/TS, simplified):
- generateContent(options)
- options.model: string
- options.contents / parts: string | Part[]
- options.config?: GenerateContentConfig (includes thinking_config / thinkingConfig / ThinkingConfig fields such as thinking_budget, thinking_level)
- options.signal / abortSignal?: AbortSignal (used to abort streaming or long requests)
Concrete references:
- thinking config usage in examples (GenerateContentConfig / ThinkingConfig). [1]
@google/genaiSDK README / API surface showing generateContent / streaming methods. [2]- SDK/type bundles using AbortSignal on request/streaming-related methods (unpkg / d.ts references). [3]
If you want, I can paste an exact typed signature and a minimal example showing how to pass ThinkingConfig and an AbortSignal for v1.35.0.
🌐 Web query:
@google/genai generateContent request shape config object fields
💡 Result:
Below are the common fields you can pass in the Node.js @google/genai generateContent "config" object (camelCased in the SDK). Not exhaustive for every backend variant, but covers the typical/most-used options:
- model (passed separately as top-level arg, not inside config) — string.
- candidateCount — number (how many response candidates to return). [1][2]
- maxOutputTokens — number (max tokens in model output). [1][2]
- temperature — number (sampling temperature). [1][2]
- topP — number (nucleus sampling). [1][2]
- topK — number (top-k sampling). [1][2]
- stopSequences — string[] (sequences where generation should stop). [1][2]
- responseMimeType — string (e.g., "text/plain" or "application/json" for structured responses). [1][2]
- responseSchema — JSON Schema object (when using responseMimeType: application/json to validate/structure output). [1][2]
- safetySettings / safetySetting — object[] / object (per-request safety controls; backend-specific names/shape vary). [2]
- tools — array (tool descriptors to enable tool/function calling). [2]
- toolConfig — object (shared tool configuration). [2]
- generationConfig / config alias — object (backend GenerationConfig for advanced model controls; SDK may expose top-level fields as above). [1][2]
- labels — map<string,string> (optional request labels for billing/telemetry on Vertex AI). [2]
References:
- Google GenAI / Gemini generateContent docs (request body and GenerationConfig examples). [1]
@google/genai(JS SDK) usage & examples (camelCase config examples, tools, responseSchema). [2]
If you want, I can paste an exact example config object for Node.js (which SDK/version you're using?)
🏁 Script executed:
# Check imports and type definitions in the file
head -50 src/core/llm/gemini.tsRepository: glowingjade/obsidian-smart-composer
Length of output: 1435
Move abortSignal to the top level of the request—it should not be in config.
The thinkingConfig field is correctly placed inside config and should remain there. However, abortSignal is a top-level request parameter in the @google/genai v1.35.0 API, not a config field. Placing it inside config will cause the abort signal to be silently ignored, breaking request cancellation.
Fix required
const result = await this.client.models.generateContent({
model: request.model,
contents: request.messages
.map((message) => GeminiProvider.parseRequestMessage(message))
.filter((m): m is Content => m !== null),
config: {
maxOutputTokens: request.max_tokens,
temperature: request.temperature,
topP: request.top_p,
presencePenalty: request.presence_penalty,
frequencyPenalty: request.frequency_penalty,
systemInstruction: systemInstruction,
- abortSignal: options?.signal,
tools: request.tools?.map((tool) =>
GeminiProvider.parseRequestTool(tool),
),
thinkingConfig: GeminiProvider.buildThinkingConfig(model),
},
+ signal: options?.signal,
})Also applies to: 108-121
🤖 Prompt for AI Agents
In `@src/core/llm/gemini.ts` around lines 81 - 101, The abortSignal
(options?.signal) is incorrectly passed inside the config object to
this.client.models.generateContent and thus ignored; move abortSignal out of
config to the top-level request parameter when calling
this.client.models.generateContent (keep thinkingConfig inside config), e.g.,
remove abortSignal from the config and add abortSignal: options?.signal
alongside model, contents, and config; apply the same change to the other
generateContent call that uses GeminiProvider.parseRequestTool /
GeminiProvider.buildThinkingConfig so cancellations work correctly.
| static parseNonStreamingResponse( | ||
| response: GenerateContentResult, | ||
| response: GenerateContentResponse, | ||
| model: string, | ||
| messageId: string, | ||
| ): LLMResponseNonStreaming { | ||
| const parts = response.candidates?.[0]?.content?.parts | ||
| const thoughtSignature = GeminiProvider.extractThoughtSignature(parts) | ||
| const reasoning = GeminiProvider.extractThoughtSummaries(parts) | ||
|
|
||
| return { | ||
| id: messageId, | ||
| choices: [ | ||
| { | ||
| finish_reason: | ||
| response.response.candidates?.[0]?.finishReason ?? null, | ||
| finish_reason: response.candidates?.[0]?.finishReason ?? null, | ||
| message: { | ||
| content: response.response.text(), | ||
| content: response.text ?? '', | ||
| reasoning: reasoning, | ||
| role: 'assistant', | ||
| tool_calls: response.response.functionCalls()?.map((f) => ({ | ||
| id: uuidv4(), | ||
| type: 'function', | ||
| function: { | ||
| name: f.name, | ||
| arguments: JSON.stringify(f.args), | ||
| }, | ||
| })), | ||
| tool_calls: GeminiProvider.parseFunctionCalls( | ||
| response.functionCalls, | ||
| ), | ||
| providerMetadata: thoughtSignature | ||
| ? { gemini: { thoughtSignature } } | ||
| : undefined, | ||
| }, | ||
| }, | ||
| ], | ||
| created: Date.now(), | ||
| model: model, | ||
| object: 'chat.completion', | ||
| usage: response.response.usageMetadata | ||
| usage: response.usageMetadata | ||
| ? { | ||
| prompt_tokens: response.response.usageMetadata.promptTokenCount, | ||
| completion_tokens: | ||
| response.response.usageMetadata.candidatesTokenCount, | ||
| total_tokens: response.response.usageMetadata.totalTokenCount, | ||
| prompt_tokens: response.usageMetadata.promptTokenCount ?? 0, | ||
| completion_tokens: response.usageMetadata.candidatesTokenCount ?? 0, | ||
| total_tokens: response.usageMetadata.totalTokenCount ?? 0, | ||
| } | ||
| : undefined, | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full gemini.ts file to understand the context
wc -l src/core/llm/gemini.tsRepository: glowingjade/obsidian-smart-composer
Length of output: 105
🏁 Script executed:
# Let's look at the complete parseNonStreamingResponse method and surrounding code
cat -n src/core/llm/gemini.ts | sed -n '280,350p'Repository: glowingjade/obsidian-smart-composer
Length of output: 2708
🏁 Script executed:
# Find the related helper methods to understand what they're doing
rg -A 10 "extractThoughtSignature|extractThoughtSummaries|parseFunctionCalls" src/core/llm/gemini.tsRepository: glowingjade/obsidian-smart-composer
Length of output: 2748
🏁 Script executed:
# Look for the GenerateContentResponse type definition
rg -B 5 "GenerateContentResponse" src/core/llm/gemini.ts | head -50Repository: glowingjade/obsidian-smart-composer
Length of output: 474
🏁 Script executed:
# Search for how the method is used to understand expected behavior
rg "parseNonStreamingResponse" --type ts -A 3 -B 3Repository: glowingjade/obsidian-smart-composer
Length of output: 3261
🏁 Script executed:
# Check the imports to understand the SDK source and version
head -50 src/core/llm/gemini.ts | grep -E "^import|@google"Repository: glowingjade/obsidian-smart-composer
Length of output: 386
🏁 Script executed:
# Look for package.json to see which version of Gemini SDK is used
cat package.json | grep -A 5 -B 5 "google\|gemini"Repository: glowingjade/obsidian-smart-composer
Length of output: 436
🏁 Script executed:
# Search for any tests or documentation files mentioning Gemini response parsing
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "README*" -o -name "*.md" \) -exec grep -l "parseNonStreamingResponse\|GenerateContentResponse" {} \;Repository: glowingjade/obsidian-smart-composer
Length of output: 61
🏁 Script executed:
# Let's look at the complete extractThoughtSummaries method to understand what it's extracting
sed -n '500,566p' src/core/llm/gemini.ts | grep -A 20 "extractThoughtSummaries"Repository: glowingjade/obsidian-smart-composer
Length of output: 61
🏁 Script executed:
# Let's see all helper methods in detail
sed -n '380,500p' src/core/llm/gemini.tsRepository: glowingjade/obsidian-smart-composer
Length of output: 3509
🌐 Web query:
Google Gemini SDK GenerateContentResponse structure response.text vs response.functionCalls consistency
💡 Result:
Short answer: the SDK can legitimately return a GenerateContentResponse with no text field when the model emits a functionCall/functionResponse part. response.text in some SDKs is a convenience accessor that assumes a text part exists and will fail or be empty in those cases — you must instead inspect candidates[].content.parts for functionCall / functionResponse and handle those branches explicitly. See the official docs and SDK issue discussion for examples and guidance. [1][2][3]
Recommended handling
- Don’t rely on response.text as the single source of truth.
- Inspect response.candidates[].content.parts in order; each Part’s union field may be text, functionCall, functionResponse, or inlineData. If you see functionCall, call your tool and append a corresponding functionResponse part back into the conversation before asking for final text. [2][3]
- Defensive code: if response.text is missing or throws, fall back to parsing parts for text or functionCall. The community-reported SDK bug shows response.text can raise when only a function call is returned. [1]
Sources
- GitHub issue reporting response.text exception when only a function call is returned. [1]
- Google AI / Gemini function-calling docs (explains parts: text, functionCall, functionResponse and recommended flow). [2]
- GenerateContentResponse schema (Vertex AI / Firebase docs showing Part union field types). [3]
🌐 Web query:
Google Gemini API thought parts response.text exclusion documentation
💡 Result:
Short answer: Gemini's docs show "thoughts" are delivered as separate content parts (part.thought boolean and thought_signature) when you enable thinking (thinkingConfig.includeThoughts). You must iterate response.candidates[0].content.parts to see thought vs non-thought text; response.text (the single flat text) may not include thought fragments. Also, Google has recently changed behavior for some models — several forum posts and docs note that thoughts may not be returned for certain model versions (or are only returned as summaries/signatures) so you may see thought content excluded from streamed response.text. [1][2][3]
Sources:
- Thought signatures / thought parts (Gemini API docs). [1]
- Thinking & multi‑step reasoning (how to enable includeThoughts and parts iteration). [2]
- Community reports / Google staff notes about thoughts being missing for some models/versions. [3]
Use response.candidates[0].content.parts as the canonical source instead of mixing response.text with response.functionCalls
Per the Gemini SDK, response.text is a convenience accessor that excludes thought parts and can be empty or unreliable when function calls are present. The current code mixes root-level response.text and response.functionCalls with candidate-level parts extraction, risking inconsistency.
Recommended fix: Extract both content text and function calls directly from response.candidates[0].content.parts by iterating through parts and classifying each by type (text, functionCall, etc.), rather than relying on response.text as a primary source.
🤖 Prompt for AI Agents
In `@src/core/llm/gemini.ts` around lines 295 - 333, In parseNonStreamingResponse,
stop mixing response.text and response.functionCalls with candidate parts;
instead treat response.candidates?.[0]?.content?.parts as the canonical source:
iterate the parts array, concatenate text-type parts into the final content
string and collect functionCall-type parts into the functionCalls structure
passed to GeminiProvider.parseFunctionCalls (or adapt parseFunctionCalls to
accept parts), then remove usage of response.text and response.functionCalls and
use the extracted content and functionCalls when building the returned choice
(preserve existing use of extractThoughtSignature and extractThoughtSummaries on
parts).
| private static readonly THINKING_LEVEL_MAP: Record<string, ThinkingLevel> = { | ||
| minimal: ThinkingLevel.MINIMAL, | ||
| low: ThinkingLevel.LOW, | ||
| medium: ThinkingLevel.MEDIUM, | ||
| high: ThinkingLevel.HIGH, | ||
| } | ||
|
|
||
| /** | ||
| * Builds the thinking config for Gemini API based on model settings. | ||
| * - Gemini 3 models use thinkingLevel | ||
| * - Gemini 2.5 models use thinkingBudget | ||
| */ | ||
| private static buildThinkingConfig( | ||
| model: ChatModel & { providerType: 'gemini' }, | ||
| ): ThinkingConfig | undefined { | ||
| if (!model.thinking?.enabled) { | ||
| return undefined | ||
| } | ||
|
|
||
| const config: ThinkingConfig = {} | ||
|
|
||
| if (model.thinking.thinking_level) { | ||
| const level = this.THINKING_LEVEL_MAP[model.thinking.thinking_level] | ||
| if (level) { | ||
| config.thinkingLevel = level | ||
| } | ||
| } | ||
|
|
||
| if (model.thinking.thinking_budget !== undefined) { | ||
| config.thinkingBudget = model.thinking.thinking_budget | ||
| } | ||
|
|
||
| if (model.thinking.include_thoughts) { | ||
| config.includeThoughts = model.thinking.include_thoughts | ||
| } | ||
|
|
||
| return config | ||
| } |
There was a problem hiding this comment.
include_thoughts should be applied when explicitly false too
Current if (model.thinking.include_thoughts) skips false. If the SDK supports explicitly disabling thought inclusion, this prevents users from turning it off once enabled by defaults.
Proposed fix
- if (model.thinking.include_thoughts) {
- config.includeThoughts = model.thinking.include_thoughts
- }
+ if (model.thinking.include_thoughts !== undefined) {
+ config.includeThoughts = model.thinking.include_thoughts
+ }📝 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.
| private static readonly THINKING_LEVEL_MAP: Record<string, ThinkingLevel> = { | |
| minimal: ThinkingLevel.MINIMAL, | |
| low: ThinkingLevel.LOW, | |
| medium: ThinkingLevel.MEDIUM, | |
| high: ThinkingLevel.HIGH, | |
| } | |
| /** | |
| * Builds the thinking config for Gemini API based on model settings. | |
| * - Gemini 3 models use thinkingLevel | |
| * - Gemini 2.5 models use thinkingBudget | |
| */ | |
| private static buildThinkingConfig( | |
| model: ChatModel & { providerType: 'gemini' }, | |
| ): ThinkingConfig | undefined { | |
| if (!model.thinking?.enabled) { | |
| return undefined | |
| } | |
| const config: ThinkingConfig = {} | |
| if (model.thinking.thinking_level) { | |
| const level = this.THINKING_LEVEL_MAP[model.thinking.thinking_level] | |
| if (level) { | |
| config.thinkingLevel = level | |
| } | |
| } | |
| if (model.thinking.thinking_budget !== undefined) { | |
| config.thinkingBudget = model.thinking.thinking_budget | |
| } | |
| if (model.thinking.include_thoughts) { | |
| config.includeThoughts = model.thinking.include_thoughts | |
| } | |
| return config | |
| } | |
| private static readonly THINKING_LEVEL_MAP: Record<string, ThinkingLevel> = { | |
| minimal: ThinkingLevel.MINIMAL, | |
| low: ThinkingLevel.LOW, | |
| medium: ThinkingLevel.MEDIUM, | |
| high: ThinkingLevel.HIGH, | |
| } | |
| /** | |
| * Builds the thinking config for Gemini API based on model settings. | |
| * - Gemini 3 models use thinkingLevel | |
| * - Gemini 2.5 models use thinkingBudget | |
| */ | |
| private static buildThinkingConfig( | |
| model: ChatModel & { providerType: 'gemini' }, | |
| ): ThinkingConfig | undefined { | |
| if (!model.thinking?.enabled) { | |
| return undefined | |
| } | |
| const config: ThinkingConfig = {} | |
| if (model.thinking.thinking_level) { | |
| const level = this.THINKING_LEVEL_MAP[model.thinking.thinking_level] | |
| if (level) { | |
| config.thinkingLevel = level | |
| } | |
| } | |
| if (model.thinking.thinking_budget !== undefined) { | |
| config.thinkingBudget = model.thinking.thinking_budget | |
| } | |
| if (model.thinking.include_thoughts !== undefined) { | |
| config.includeThoughts = model.thinking.include_thoughts | |
| } | |
| return config | |
| } |
🤖 Prompt for AI Agents
In `@src/core/llm/gemini.ts` around lines 505 - 542, In buildThinkingConfig
(class-level THINKING_LEVEL_MAP and method buildThinkingConfig), the check "if
(model.thinking.include_thoughts)" ignores explicit false; change the gating to
test for undefined (e.g., "if (model.thinking.include_thoughts !== undefined)")
so includeThoughts is set when the caller explicitly passes false or true; keep
the rest of the mapping for thinkingLevel/thinkingBudget unchanged.
kevin-on
changed the title
1 participant
Summary
This PR introduces several provider-related improvements: adds xAI (Grok) as a first-class provider, upgrades the Gemini SDK from
@google/generative-aito@google/genai, and addsproviderMetadatato preserve thinking/reasoning context across multi-turn conversations with tool calls.Changes
New Provider: xAI
XaiProviderwith support for Grok models (grok-4-1-fast,grok-4-1-fast-non-reasoning)XAI_PRICESto the price calculatorDeprecated Providers: Groq & Morph
GroqProviderandMorphProvideras dedicated providersopenai-compatibletype with appropriate base URLsGemini SDK Upgrade
@google/generative-ai(v0.24.0) to@google/genai(v1.35.0)thinking_level(minimal/low/medium/high) for Gemini 3 modelsthinking_budgetfor Gemini 2.5 modelsinclude_thoughtsto return thought summariesProvider Metadata for Thinking Context
providerMetadatafield to chat messages and LLM request/response typesthoughtSignatureto maintain thinking continuity across tool call iterationsreasoningContentto pass back reasoning in assistant messages during tool call loopsModel Updates
gpt-5.2,gpt-4.1-mini,claude-opus-4.5,gemini-3-pro-preview,gemini-3-flash-preview,grok-4-1-fast,grok-4-1-fast-non-reasoningOther Improvements
Checklist before requesting a review
npm run lint:checkandnpm run type:check)npm run test)Summary by CodeRabbit
New Features
Migrations
Improvements
Pricing
Tests
✏️ Tip: You can customize this high-level summary in your review settings.