feat: add llmMetadata to inference connection - #1426
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 20 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR extends inference connections by adding optional Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/renderer/src/lib/chat/components/chat.svelte (1)
4-14:⚠️ Potential issue | 🟠 MajorKeep model matching backward-compatible when
providerNameis absent.
providerNameis optional, and persistedLAST_USED_MODEL_KEYvalues written before this PR will not have it. With strict equality, those entries no longer match models that now includeproviderName, causing the UI to fall back tomodels[0]and potentially switch providers/models silently.🐛 Proposed fix
return models.find( m => m.label === model.label && m.providerId === model.providerId && m.connectionName === model.connectionName && m.type === model.type && - m.providerName === model.providerName && + (model.providerName === undefined || m.providerName === model.providerName) && m.endpoint === model.endpoint, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/lib/chat/components/chat.svelte` around lines 4 - 14, The equality check in findModel is too strict for optional providerName and breaks matching persisted LAST_USED_MODEL_KEY entries that lack providerName; update the comparison in findModel so providerName is treated as a backward-compatible wildcard (i.e., consider it a match when the stored model's providerName is undefined or when both providerName values are equal) while leaving the other field comparisons unchanged so legacy entries still match current models.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/ramalama/src/manager/inference-model-manager.ts`:
- Line 61: The providerName field in the provider registration is set to
'openai' but should match the extension's provider.id; change providerName:
'openai' to providerName: 'ramalama' in the inference-model-manager.ts
registration (the providerName property) so it follows the same pattern used by
other extensions like 'ollama', 'mistral', and 'gemini'.
In `@packages/renderer/src/lib/models/models-utils.ts`:
- Around line 8-15: Legacy saved model entries lacking providerName break the
strict matcher in chat.svelte's findModel; update the matcher to accept
undefined providerName as a wildcard (i.e., treat a saved model with
selectedModel.providerName === undefined as matching any m.providerName) or add
a fallback match checking providerId+connectionName+label when providerName is
missing. Locate the comparator in findModel/selectedModel.providerName and
change the equality to allow model.providerName === undefined OR include the
fallback condition, and/or ensure the model list built in models-utils (the
accumulator.push ... models.map block that sets providerId, connectionName,
label) backfills providerName into legacy entries so matches succeed.
---
Outside diff comments:
In `@packages/renderer/src/lib/chat/components/chat.svelte`:
- Around line 4-14: The equality check in findModel is too strict for optional
providerName and breaks matching persisted LAST_USED_MODEL_KEY entries that lack
providerName; update the comparison in findModel so providerName is treated as a
backward-compatible wildcard (i.e., consider it a match when the stored model's
providerName is undefined or when both providerName values are equal) while
leaving the other field comparisons unchanged so legacy entries still match
current models.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab4113e2-5daa-46ac-a253-ec77f24afea4
📒 Files selected for processing (17)
extensions/gemini/src/gemini.spec.tsextensions/gemini/src/gemini.tsextensions/mistral/src/manager/mistral-inference-manager.spec.tsextensions/mistral/src/manager/mistral-inference-manager.tsextensions/ollama/src/ollama-extension.tsextensions/openai-compatible/src/openAI.spec.tsextensions/openai-compatible/src/openAI.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/ramalama/src/manager/inference-model-manager.tspackages/api/src/chat/message-config.tspackages/api/src/provider-info.tspackages/extension-api/src/extension-api.d.tspackages/main/src/chat/chat-manager.tspackages/main/src/plugin/provider-registry.tspackages/renderer/src/lib/chat/components/chat.sveltepackages/renderer/src/lib/chat/components/model-info.tspackages/renderer/src/lib/models/models-utils.ts
📜 Review details
⏰ 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). (10)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: unit tests / windows-2025
- GitHub Check: typecheck
- GitHub Check: Linux
- GitHub Check: linter, formatters
- GitHub Check: Windows
- GitHub Check: macOS
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases (e.g.,'/@/plugin/provider-registry.js') instead of relative paths (e.g.,'../plugin/provider-registry.js') for imports outside the current directory's module group. Relative imports are only used for sibling modules within the same directory.
Files:
extensions/ramalama/src/manager/inference-model-manager.tspackages/api/src/chat/message-config.tsextensions/mistral/src/manager/mistral-inference-manager.spec.tsextensions/ollama/src/ollama-extension.tspackages/main/src/chat/chat-manager.tsextensions/mistral/src/manager/mistral-inference-manager.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/openai-compatible/src/openAI.tsextensions/openai-compatible/src/openAI.spec.tsextensions/gemini/src/gemini.spec.tspackages/renderer/src/lib/models/models-utils.tspackages/api/src/provider-info.tspackages/main/src/plugin/provider-registry.tsextensions/gemini/src/gemini.tspackages/renderer/src/lib/chat/components/model-info.tspackages/extension-api/src/extension-api.d.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in Vitest. Avoid manual mock factories (vi.mock('...', () => ({...}))) when possible
Usevi.resetAllMocks()inbeforeEachfor resetting mocks in Vitest unit tests, notvi.clearAllMocks()
When an auto-mocked function or class method needs a real implementation in Vitest, usevi.mocked(...). For class methods, use the prototype pattern:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
extensions/mistral/src/manager/mistral-inference-manager.spec.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/openai-compatible/src/openAI.spec.tsextensions/gemini/src/gemini.spec.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: Long-running operations should useTaskManagerwithcreateTask()to provide user feedback on operation status
Container operations in the main process must useContainerProviderRegistrywith theengineIdparameter to identify the container engine
Kubernetes operations in the main process should useKubernetesClientfor context management, resource operations, port forwarding, and exec operations
IPC handlers in the main process must follow the naming convention:<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
Store credentials and sensitive setup data securely viaSafeStorageRegistryinstead of plain configuration
Files:
packages/main/src/chat/chat-manager.tspackages/main/src/plugin/provider-registry.ts
packages/renderer/src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
External URLs in the renderer process require user confirmation (handled via
setupSecurityRestrictionsOnLinks)
Files:
packages/renderer/src/lib/models/models-utils.tspackages/renderer/src/lib/chat/components/chat.sveltepackages/renderer/src/lib/chat/components/model-info.ts
packages/renderer/src/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte component development guidelines including color-registry usage and Icon component as specified in CODE-GUIDELINES.md and
.agents/skills/ui-components/
Files:
packages/renderer/src/lib/chat/components/chat.svelte
🧠 Learnings (11)
📓 Common learnings
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/models-utils.ts:11-20
Timestamp: 2026-04-17T20:26:32.946Z
Learning: In `packages/renderer/src/lib/models/models-utils.ts` (openkaiden/kaiden), `InferenceConnectionSummary.connectionType` is intentionally optional. It is only `undefined` for the single synthetic `'not-configured'` entry (emitted when a provider has `inferenceProviderConnectionCreation` but no active `inferenceConnections`). All consumers guard with optional chaining. A discriminated union was considered but deferred as unnecessary complexity for v1, since the invariant (`connectionType` is defined iff `status !== 'not-configured'`) is self-evident from the `status` field. Do not flag this as a type-safety issue.
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1295
File: packages/extension-api/src/extension-api.d.ts:651-651
Timestamp: 2026-04-13T15:59:29.742Z
Learning: In the openkaiden/kaiden repository, the project relies on TypeScript's static type checking (not runtime validation) to enforce type correctness for `InferenceProviderConnection.type` (`InferenceProviderConnectionType`). Runtime normalization/validation guards for this field are not needed or desired.
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T04:24:07.577Z
Learning: Applies to extensions/*/package.json : Extensions should declare provider capabilities (inference providers, flow providers, MCP registries, configuration properties) in the `contributes` section of their `package.json`
📚 Learning: 2026-04-17T20:26:32.946Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/models-utils.ts:11-20
Timestamp: 2026-04-17T20:26:32.946Z
Learning: In `packages/renderer/src/lib/models/models-utils.ts` (openkaiden/kaiden), `InferenceConnectionSummary.connectionType` is intentionally optional. It is only `undefined` for the single synthetic `'not-configured'` entry (emitted when a provider has `inferenceProviderConnectionCreation` but no active `inferenceConnections`). All consumers guard with optional chaining. A discriminated union was considered but deferred as unnecessary complexity for v1, since the invariant (`connectionType` is defined iff `status !== 'not-configured'`) is self-evident from the `status` field. Do not flag this as a type-safety issue.
Applied to files:
extensions/ramalama/src/manager/inference-model-manager.tsextensions/mistral/src/manager/mistral-inference-manager.spec.tspackages/main/src/chat/chat-manager.tsextensions/mistral/src/manager/mistral-inference-manager.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/openai-compatible/src/openAI.tsextensions/openai-compatible/src/openAI.spec.tsextensions/gemini/src/gemini.spec.tspackages/renderer/src/lib/models/models-utils.tspackages/api/src/provider-info.tspackages/main/src/plugin/provider-registry.tsextensions/gemini/src/gemini.tspackages/renderer/src/lib/chat/components/model-info.tspackages/extension-api/src/extension-api.d.ts
📚 Learning: 2026-04-13T15:59:29.742Z
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1295
File: packages/extension-api/src/extension-api.d.ts:651-651
Timestamp: 2026-04-13T15:59:29.742Z
Learning: In the openkaiden/kaiden repository, the project relies on TypeScript's static type checking (not runtime validation) to enforce type correctness for `InferenceProviderConnection.type` (`InferenceProviderConnectionType`). Runtime normalization/validation guards for this field are not needed or desired.
Applied to files:
extensions/mistral/src/manager/mistral-inference-manager.spec.tsextensions/mistral/src/manager/mistral-inference-manager.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/openai-compatible/src/openAI.tsextensions/openai-compatible/src/openAI.spec.tsextensions/gemini/src/gemini.spec.tspackages/api/src/provider-info.tspackages/main/src/plugin/provider-registry.tsextensions/gemini/src/gemini.tspackages/renderer/src/lib/chat/components/model-info.tspackages/extension-api/src/extension-api.d.ts
📚 Learning: 2026-04-16T04:24:07.577Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T04:24:07.577Z
Learning: Applies to extensions/*/package.json : Extensions should declare provider capabilities (inference providers, flow providers, MCP registries, configuration properties) in the `contributes` section of their `package.json`
Applied to files:
extensions/ollama/src/ollama-extension.tsextensions/mistral/src/manager/mistral-inference-manager.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/openai-compatible/src/openAI.tspackages/api/src/provider-info.tspackages/main/src/plugin/provider-registry.tsextensions/gemini/src/gemini.tspackages/renderer/src/lib/chat/components/model-info.tspackages/extension-api/src/extension-api.d.ts
📚 Learning: 2026-04-20T14:30:15.867Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts:30-45
Timestamp: 2026-04-20T14:30:15.867Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts`, the `stubOllama` helper intentionally only re-stubs `fetch` and relies on `beforeEach`'s `stubRamalama(false)` call to keep `getProviderInfos` in place. This is a deliberate standard Vitest `beforeEach` + per-test override pattern. Do not flag the implicit dependency between `stubOllama` and the `beforeEach` ramalama stub as a robustness issue.
Applied to files:
extensions/ramalama/src/manager/inference-model-manager.spec.ts
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/provider-registry.ts
📚 Learning: 2026-04-17T20:26:55.521Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/ModelsCatalog.svelte:51-53
Timestamp: 2026-04-17T20:26:55.521Z
Learning: In `packages/renderer/src/lib/models/ModelsCatalog.svelte` (openkaiden/kaiden), `ModelSelectable = CatalogModelInfo & { selected: boolean }` is intentionally defined and `selected: false` is set on each filtered row. The `selected` field is structurally required by the `podman-desktop/ui-svelte` `Table` component's generic constraint (`T extends { selected?: boolean; name?: string }`). Removing it causes TypeScript type errors. Do not flag this as dead state.
Applied to files:
packages/renderer/src/lib/chat/components/chat.svelte
📚 Learning: 2026-04-17T20:27:11.322Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/stores/model-catalog.ts:26-28
Timestamp: 2026-04-17T20:27:11.322Z
Learning: In `packages/renderer/src/stores/model-catalog.ts`, provider IDs used as the first component of `modelKey(providerId, label)` are always simple colon-free slug strings (e.g. `gemini`, `claude`, `openai`, `openshiftai`). The `:` separator in `modelKey` does not risk key collisions because provider IDs are guaranteed never to contain a colon by convention. Do not flag this as a collision risk in future reviews.
Applied to files:
packages/renderer/src/lib/chat/components/chat.sveltepackages/renderer/src/lib/chat/components/model-info.ts
📚 Learning: 2026-04-02T14:47:51.059Z
Learnt from: fbricon
Repo: kortex-hub/kortex PR: 1196
File: packages/renderer/src/lib/chat/components/chat.svelte:244-265
Timestamp: 2026-04-02T14:47:51.059Z
Learning: In the chat UI, calling `chatHistory.refetch()` (from `ChatHistory.fromContext()`) is only intended to refresh the sidebar chat history list and will not reload the conversation’s displayed chat messages. If you need to reload persisted chat messages for a given `chatId`, call `window.inferenceGetChatMessagesById(chatId)` directly instead of relying on `chatHistory.refetch()`.
Applied to files:
packages/renderer/src/lib/chat/components/chat.svelte
📚 Learning: 2026-04-15T08:04:32.031Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.svelte:9-11
Timestamp: 2026-04-15T08:04:32.031Z
Learning: For Svelte components in this repo, if a callback prop is typed as `() => void`, TypeScript idiomatically allows passing async functions (e.g., `() => Promise<void>`), because `() => void` indicates the caller ignores the return value rather than requiring `undefined`. Do not recommend changing these prop types to `() => void | Promise<void>` solely to “fix” async compatibility—unless there is an actual need for the caller to observe the returned value.
Applied to files:
packages/renderer/src/lib/chat/components/chat.svelte
📚 Learning: 2026-04-16T04:24:07.577Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T04:24:07.577Z
Learning: Applies to extensions/*/package.json : Configuration properties must be defined in extension's `package.json` under `contributes.configuration.properties` with appropriate scopes (DEFAULT, ContainerProviderConnection, KubernetesProviderConnection, InferenceProviderConnection, InferenceProviderConnectionFactory)
Applied to files:
packages/extension-api/src/extension-api.d.ts
🔇 Additional comments (10)
extensions/gemini/src/gemini.ts (1)
158-158: LGTM!
providerName: 'gemini'is consistent with the provider id and matches the optionalproviderName?: stringfield onInferenceProviderConnection.extensions/openai-compatible/src/openAI.spec.ts (1)
181-181: LGTM!Assertion correctly mirrors the production change adding
providerName: 'openai'.extensions/mistral/src/manager/mistral-inference-manager.spec.ts (1)
164-164: LGTM!Exact-match expectation on the registration payload correctly includes
providerName: 'mistral'.extensions/ollama/src/ollama-extension.ts (1)
104-104: LGTM!
providerName: 'ollama'matches the provider id and is consistent with other cloud/local extensions in this PR.extensions/gemini/src/gemini.spec.ts (1)
200-200: LGTM!Test expectation aligned with
gemini.tsaddingproviderName: 'gemini'.extensions/openai-compatible/src/openAI.ts (1)
190-190: LGTM!
providerName: 'openai'matches the provider id; addition is consistent with the optionalproviderName?: stringfield defined inpackages/extension-api/src/extension-api.d.ts.extensions/mistral/src/manager/mistral-inference-manager.ts (1)
111-111: LGTM!
providerName: 'mistral'is consistent with the provider id and satisfies the optional field onInferenceProviderConnection.packages/main/src/plugin/provider-registry.ts (2)
790-799: LGTM: inference connection info now carries provider metadata.This correctly forwards the optional
providerNamealongside the existing inference connection fields.
2049-2060: LGTM: provider-name accessor matches the existing inference getters.The method preserves the optional return type and uses the same provider/connection lookup pattern as the neighboring accessors.
packages/renderer/src/lib/chat/components/chat.svelte (1)
60-70: LGTM: selected model now preserves configured provider metadata.Including
config.providerNamekeeps stored chat configuration aligned with the expanded model identity.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
benoitf
left a comment
benoitf
left a comment
There was a problem hiding this comment.
I was wondering why the extension would need to provide the name of the provider as we're calling a method on the provider
I think from client side/extension side we don't change the register method, but then it's when we grab the connection object that we can have this extra field coming from its parent the provider
Because here I could call registerConnection with a different provider name than the one we have
|
I think it's like for but we add providerName in addition of providerId (I would assume that you might still find the provider's name by it's providerId as well no) ? for inferenceConnections it would be there: |
Yes but here we're dealing with other notion of providers: there are the LLM providers so the ramalama extension might specify openai as the providerName there as it's likely how it will be seen from the agent POV |
|
yes please use another name then. Also I think it might be a free-form entry because you may want to add many fields maybe llmMetadata and llmMetadata can have name, version, whatever |
jeffmaury
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/extension-api/src/extension-api.d.ts`:
- Around line 653-655: The LLMMetadata interface is too restrictive (only name?:
string) so extensions can't add richer metadata; update the LLMMetadata
declaration (interface LLMMetadata) to be extensible by adding either an index
signature (e.g., [key: string]: any) or explicit optional fields you expect
(e.g., version?: string, provider?: string) so callers can attach extra
properties without type errors; modify the interface definition accordingly
(keep name?: string) and ensure the chosen shape is used wherever LLMMetadata is
referenced.
In `@packages/main/src/plugin/provider-registry.ts`:
- Around line 2049-2060: Rename the method getInferenceConnectionProviderName to
getInferenceConnectionLLMMetadata and update its signature/return type usage:
change the function name in the provider-registry class
(getInferenceConnectionProviderName -> getInferenceConnectionLLMMetadata),
update all internal and external callers, exports, and any interface/type
references that refer to getInferenceConnectionProviderName, and ensure tests
and usages expecting a string are adjusted since the method returns
InferenceProviderConnection['llmMetadata'] (an object); keep the implementation
unchanged except for the new name so it still fetches provider via
getMatchingProviderInternalId, finds the connection by name, and returns
connection.llmMetadata.
In `@stop-hook.txt`:
- Around line 1-6: Remove the generated local session transcript file from the
PR by deleting stop-hook.txt from the commit and ensuring it's ignored going
forward; delete the file (stop-hook.txt) and add a pattern to the appropriate
.gitignore (or the repo's ignore config) to exclude local session
artifacts/session transcripts so this file won’t be re-added.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 201ed8bf-1afd-4ffa-916a-03ad3005ad6c
📒 Files selected for processing (15)
extensions/gemini/src/gemini.spec.tsextensions/gemini/src/gemini.tsextensions/mistral/src/manager/mistral-inference-manager.spec.tsextensions/mistral/src/manager/mistral-inference-manager.tsextensions/ollama/src/ollama-extension.tsextensions/openai-compatible/src/openAI.spec.tsextensions/openai-compatible/src/openAI.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/ramalama/src/manager/inference-model-manager.tspackages/api/src/provider-info.tspackages/extension-api/src/extension-api.d.tspackages/main/src/plugin/provider-registry.tspackages/renderer/src/lib/chat/components/model-info.tspackages/renderer/src/lib/models/models-utils.tsstop-hook.txt
📜 Review details
⏰ 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). (10)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: macOS
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: typecheck
- GitHub Check: Linux
- GitHub Check: linter, formatters
- GitHub Check: unit tests / windows-2025
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases (e.g.,'/@/plugin/provider-registry.js') instead of relative paths (e.g.,'../plugin/provider-registry.js') for imports outside the current directory's module group. Relative imports are only used for sibling modules within the same directory.
Files:
extensions/mistral/src/manager/mistral-inference-manager.tsextensions/gemini/src/gemini.tsextensions/openai-compatible/src/openAI.tspackages/api/src/provider-info.tspackages/renderer/src/lib/models/models-utils.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/gemini/src/gemini.spec.tsextensions/openai-compatible/src/openAI.spec.tspackages/main/src/plugin/provider-registry.tspackages/extension-api/src/extension-api.d.tsextensions/ollama/src/ollama-extension.tsextensions/ramalama/src/manager/inference-model-manager.tsextensions/mistral/src/manager/mistral-inference-manager.spec.tspackages/renderer/src/lib/chat/components/model-info.ts
packages/renderer/src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
External URLs in the renderer process require user confirmation (handled via
setupSecurityRestrictionsOnLinks)
Files:
packages/renderer/src/lib/models/models-utils.tspackages/renderer/src/lib/chat/components/model-info.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in Vitest. Avoid manual mock factories (vi.mock('...', () => ({...}))) when possible
Usevi.resetAllMocks()inbeforeEachfor resetting mocks in Vitest unit tests, notvi.clearAllMocks()
When an auto-mocked function or class method needs a real implementation in Vitest, usevi.mocked(...). For class methods, use the prototype pattern:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
extensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/gemini/src/gemini.spec.tsextensions/openai-compatible/src/openAI.spec.tsextensions/mistral/src/manager/mistral-inference-manager.spec.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: Long-running operations should useTaskManagerwithcreateTask()to provide user feedback on operation status
Container operations in the main process must useContainerProviderRegistrywith theengineIdparameter to identify the container engine
Kubernetes operations in the main process should useKubernetesClientfor context management, resource operations, port forwarding, and exec operations
IPC handlers in the main process must follow the naming convention:<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
Store credentials and sensitive setup data securely viaSafeStorageRegistryinstead of plain configuration
Files:
packages/main/src/plugin/provider-registry.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1295
File: packages/extension-api/src/extension-api.d.ts:651-651
Timestamp: 2026-04-13T15:59:29.742Z
Learning: In the openkaiden/kaiden repository, the project relies on TypeScript's static type checking (not runtime validation) to enforce type correctness for `InferenceProviderConnection.type` (`InferenceProviderConnectionType`). Runtime normalization/validation guards for this field are not needed or desired.
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/models-utils.ts:11-20
Timestamp: 2026-04-17T20:26:32.946Z
Learning: In `packages/renderer/src/lib/models/models-utils.ts` (openkaiden/kaiden), `InferenceConnectionSummary.connectionType` is intentionally optional. It is only `undefined` for the single synthetic `'not-configured'` entry (emitted when a provider has `inferenceProviderConnectionCreation` but no active `inferenceConnections`). All consumers guard with optional chaining. A discriminated union was considered but deferred as unnecessary complexity for v1, since the invariant (`connectionType` is defined iff `status !== 'not-configured'`) is self-evident from the `status` field. Do not flag this as a type-safety issue.
📚 Learning: 2026-04-17T20:26:32.946Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/models-utils.ts:11-20
Timestamp: 2026-04-17T20:26:32.946Z
Learning: In `packages/renderer/src/lib/models/models-utils.ts` (openkaiden/kaiden), `InferenceConnectionSummary.connectionType` is intentionally optional. It is only `undefined` for the single synthetic `'not-configured'` entry (emitted when a provider has `inferenceProviderConnectionCreation` but no active `inferenceConnections`). All consumers guard with optional chaining. A discriminated union was considered but deferred as unnecessary complexity for v1, since the invariant (`connectionType` is defined iff `status !== 'not-configured'`) is self-evident from the `status` field. Do not flag this as a type-safety issue.
Applied to files:
extensions/mistral/src/manager/mistral-inference-manager.tsextensions/gemini/src/gemini.tsextensions/openai-compatible/src/openAI.tspackages/api/src/provider-info.tspackages/renderer/src/lib/models/models-utils.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/gemini/src/gemini.spec.tsextensions/openai-compatible/src/openAI.spec.tspackages/main/src/plugin/provider-registry.tspackages/extension-api/src/extension-api.d.tsextensions/ramalama/src/manager/inference-model-manager.tspackages/renderer/src/lib/chat/components/model-info.ts
📚 Learning: 2026-04-13T15:59:29.742Z
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1295
File: packages/extension-api/src/extension-api.d.ts:651-651
Timestamp: 2026-04-13T15:59:29.742Z
Learning: In the openkaiden/kaiden repository, the project relies on TypeScript's static type checking (not runtime validation) to enforce type correctness for `InferenceProviderConnection.type` (`InferenceProviderConnectionType`). Runtime normalization/validation guards for this field are not needed or desired.
Applied to files:
packages/api/src/provider-info.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/openai-compatible/src/openAI.spec.tspackages/main/src/plugin/provider-registry.tspackages/extension-api/src/extension-api.d.tspackages/renderer/src/lib/chat/components/model-info.ts
📚 Learning: 2026-04-17T20:27:11.322Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/stores/model-catalog.ts:26-28
Timestamp: 2026-04-17T20:27:11.322Z
Learning: In `packages/renderer/src/stores/model-catalog.ts`, provider IDs used as the first component of `modelKey(providerId, label)` are always simple colon-free slug strings (e.g. `gemini`, `claude`, `openai`, `openshiftai`). The `:` separator in `modelKey` does not risk key collisions because provider IDs are guaranteed never to contain a colon by convention. Do not flag this as a collision risk in future reviews.
Applied to files:
packages/renderer/src/lib/models/models-utils.tsextensions/ramalama/src/manager/inference-model-manager.ts
📚 Learning: 2026-04-14T13:16:08.886Z
Learnt from: fbricon
Repo: openkaiden/kaiden PR: 1332
File: packages/renderer/src/stores/chat-window.ts:40-43
Timestamp: 2026-04-14T13:16:08.886Z
Learning: In `packages/renderer/src/stores/chat-window.ts` (openkaiden/kaiden), the `showChatWindow` store intentionally uses `showChatWindow.set(value === true)`, mapping `undefined` to `false`. The `chat.showChatWindow` setting has `default: false` in the schema (`packages/main/src/plugin/chat-init.ts`), making the chat window **opt-in** (hidden by default). Do not flag `value === true` as a bug — the chat should only be visible when the config value is explicitly `true`.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-17T20:26:55.521Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/ModelsCatalog.svelte:51-53
Timestamp: 2026-04-17T20:26:55.521Z
Learning: In `packages/renderer/src/lib/models/ModelsCatalog.svelte` (openkaiden/kaiden), `ModelSelectable = CatalogModelInfo & { selected: boolean }` is intentionally defined and `selected: false` is set on each filtered row. The `selected` field is structurally required by the `podman-desktop/ui-svelte` `Table` component's generic constraint (`T extends { selected?: boolean; name?: string }`). Removing it causes TypeScript type errors. Do not flag this as dead state.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-17T20:26:14.460Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/columns/ModelStatusColumn.svelte:15-24
Timestamp: 2026-04-17T20:26:14.460Z
Learning: In `packages/renderer/src/lib/models/columns/ModelStatusColumn.svelte`, the `statusMap` intentionally maps `unknown` → `'RUNNING'` because the Gemini extension reports `unknown` connection status even when the connection is healthy (API key accepted, models loaded). Flagging this as misleading is incorrect; changing it to `DEGRADED` would produce a false warning for healthy Gemini providers. The root fix (extensions should report `started`) is out of scope for UI-only PRs.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-22T02:58:56.754Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/ModelsCatalogEmptyScreen.spec.ts:32-38
Timestamp: 2026-04-22T02:58:56.754Z
Learning: In `openkaiden/kaiden`, test files under `packages/renderer/src/lib/models/**/*.spec.ts` (and sibling spec files) intentionally use exact user-facing copy strings in `screen.getByText(...)` assertions rather than regex matchers or `data-testid`. This is a deliberate Testing Library convention: if the wording changes, the test should fail to prompt an intentional update. Do not flag exact-string `getByText` assertions as brittle in these test files.
Applied to files:
packages/renderer/src/lib/models/models-utils.tsextensions/openai-compatible/src/openAI.spec.ts
📚 Learning: 2026-04-23T11:33:39.165Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1431
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte:43-52
Timestamp: 2026-04-23T11:33:39.165Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte` and `packages/renderer/src/lib/guided-setup/guided-setup-steps.ts` (openkaiden/kaiden), `CliAgent` is the source-of-truth union type representing agent names supported by the `kdn` CLI (e.g., used by `kdn init --agent`). The `agentDefinitions` registry in `agent-registry.ts` is always a UI-side subset that must conform to `CliAgent`, not define it. Do not suggest deriving `CliAgent` from the registry — that would invert the intended dependency direction. The `as CliAgent` cast in `CodingAgentStep.svelte` is safe by construction because `agentDefinitions[].cliName` is typed as `CliAgent`.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-21T09:42:09.739Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte:126-143
Timestamp: 2026-04-21T09:42:09.739Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte`, the UI copy referencing "the next step" (e.g., "Results update the default-model step." and "You can pick a default from the local catalog on the next step.") is intentionally forward-looking. A follow-up issue will add a model-selection step after the coding-agent step in the onboarding wizard. Do not flag these strings as referencing a non-existent step.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-22T09:55:01.642Z
Learnt from: fbricon
Repo: openkaiden/kaiden PR: 1418
File: packages/renderer/src/stores/navigation/navigation-registry-agent-workspaces.svelte.ts:25-28
Timestamp: 2026-04-22T09:55:01.642Z
Learning: In `packages/renderer/src/stores/navigation/navigation-registry-agent-workspaces.svelte.ts` (openkaiden/kaiden), the navigation sidebar label `name: 'Workspaces'` is intentionally short (not 'Agentic Workspaces'). This matches the approved design mockup. The full label is surfaced in the page title and tooltip ('Agentic Workspaces'). Do not flag the terse sidebar label as ambiguous or suggest expanding it to 'Agentic Workspaces'.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-22T02:58:53.602Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/columns/ModelSizeColumn.spec.ts:19-41
Timestamp: 2026-04-22T02:58:53.602Z
Learning: In `packages/renderer/src/lib/models/columns/ModelSizeColumn.spec.ts` (openkaiden/kaiden), the team intentionally asserts the literal em-dash glyph (`—`) rendered by `ModelSizeColumn.svelte`. This is deliberate Testing Library practice: test what the user sees, and let the test break if the placeholder glyph ever changes as a change-detection signal. Do not suggest replacing this with `data-testid` or role-based queries, as that would be contrary to the project's testing philosophy.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-20T14:31:10.155Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts:114-123
Timestamp: 2026-04-20T14:31:10.155Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts`, the team's convention is that a test's name plus its stub setup lines serve as sufficient documentation of intent; inline comments restating what the stubs do are considered redundant and are intentionally omitted. Do not flag the absence of such comments as a clarity issue.
Applied to files:
extensions/openai-compatible/src/openAI.spec.tsstop-hook.txt
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/provider-registry.ts
📚 Learning: 2026-04-23T04:28:25.544Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1417
File: extensions/kdn/vite.config.js:60-62
Timestamp: 2026-04-23T04:28:25.544Z
Learning: In openkaiden/kaiden, the `__mocks__/` directory (containing `vitest-generate-api-global-setup.ts` and `openkaiden/api.js`) lives at the repository root, NOT under `extensions/`. Extensions (e.g., `extensions/kdn`, `extensions/gemini`) use `join(PACKAGE_ROOT, '..', '..', '__mocks__', ...)` in their `vite.config.js` to correctly resolve to the repo-root `__mocks__/` folder. Do not flag this two-level-up path traversal as incorrect in future reviews.
Applied to files:
stop-hook.txt
📚 Learning: 2026-04-23T04:28:18.818Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1417
File: extensions/kdn/scripts/build.js:42-46
Timestamp: 2026-04-23T04:28:18.818Z
Learning: In openkaiden/kaiden, all extension `scripts/build.js` files intentionally extract the `.cdix` zip into a `builtin/<name>.cdix/` directory (not keep it as a zip archive). The electron-builder config at `.electron-builder.config.cjs` picks this up via the glob `'extensions/**/builtin/*.cdix/**'`. Do not flag this extract-to-directory pattern as incorrect in future reviews — it is the established convention used by all extensions (claude, docling, gemini, kdn, mcp-registries, milvus, mistral, ollama, openai-compatible, openshift-ai, ramalama).
Applied to files:
stop-hook.txt
📚 Learning: 2026-04-16T04:24:07.577Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T04:24:07.577Z
Learning: Applies to extensions/*/package.json : Extensions should declare provider capabilities (inference providers, flow providers, MCP registries, configuration properties) in the `contributes` section of their `package.json`
Applied to files:
packages/extension-api/src/extension-api.d.tsextensions/ramalama/src/manager/inference-model-manager.ts
📚 Learning: 2026-04-20T14:30:15.867Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts:30-45
Timestamp: 2026-04-20T14:30:15.867Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts`, the `stubOllama` helper intentionally only re-stubs `fetch` and relies on `beforeEach`'s `stubRamalama(false)` call to keep `getProviderInfos` in place. This is a deliberate standard Vitest `beforeEach` + per-test override pattern. Do not flag the implicit dependency between `stubOllama` and the `beforeEach` ramalama stub as a robustness issue.
Applied to files:
extensions/ramalama/src/manager/inference-model-manager.ts
🔇 Additional comments (13)
packages/api/src/provider-info.ts (1)
19-30: LGTM.The API DTO now carries optional
llmMetadataconsistently with the extension API type while preserving backward compatibility.Also applies to: 74-85
extensions/openai-compatible/src/openAI.ts (1)
187-205: LGTM.The OpenAI-compatible connection now advertises the expected LLM metadata alongside the existing endpoint, SDK, models, and credentials.
extensions/gemini/src/gemini.ts (1)
155-172: LGTM.The Gemini connection now includes the expected provider metadata without changing registration behavior.
extensions/ramalama/src/manager/inference-model-manager.ts (1)
58-71: LGTM.Using
openaihere is consistent with the OpenAI-compatible SDK endpoint exposed by Ramalama.extensions/mistral/src/manager/mistral-inference-manager.ts (1)
108-125: LGTM.The Mistral connection now carries the expected LLM metadata while preserving the existing lifecycle and credential behavior.
extensions/openai-compatible/src/openAI.spec.ts (1)
176-188: LGTM.The test now verifies that OpenAI-compatible inference connections include the new metadata field.
extensions/ollama/src/ollama-extension.ts (1)
101-114: LGTM.The Ollama connection now carries the expected LLM metadata without changing connection lifecycle behavior.
extensions/mistral/src/manager/mistral-inference-manager.spec.ts (1)
160-172: LGTM.The expectation now verifies that Mistral registrations include the new
llmMetadatafield.extensions/gemini/src/gemini.spec.ts (1)
196-208: LGTM.The test now covers the Gemini
llmMetadataregistration field.extensions/ramalama/src/manager/inference-model-manager.spec.ts (1)
69-78: LGTM.The assertion correctly verifies OpenAI-compatible LLM metadata for RamaLama model registrations.
packages/main/src/plugin/provider-registry.ts (1)
790-799: LGTM.Inference connection info now exposes
llmMetadataalongside the existing model, endpoint, type, and status fields.packages/renderer/src/lib/chat/components/model-info.ts (1)
19-25: LGTM.
ModelInfonow has the optional metadata field needed by the renderer without making existing model entries incompatible.packages/renderer/src/lib/models/models-utils.ts (1)
4-24: LGTM.
getModels()now preserves LLM metadata from inference connections when building rendererModelInfoentries.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
extensions/openai-compatible/src/openAI.ts (1)
187-205:⚠️ Potential issue | 🟡 MinorConsider whether
llmMetadata.name: 'openai'accurately represents all backends.The
openai-compatibleextension can register connections against any OpenAI-compatible endpoint (Azure OpenAI, local proxies, third-party gateways, etc.), but every such connection will be reported to agents asllmMetadata.name: 'openai'. If downstream agents use this value to infer model capabilities, pricing, or tokenization, they may be misled for non-OpenAI backends.Since the PR discussion settled on
llmMetadataas a free-form bag, consider either deriving the name from thebaseURL(when recognizable) or documenting thatname: 'openai'denotes the wire protocol, not the upstream provider.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/openai-compatible/src/openAI.ts` around lines 187 - 205, The llmMetadata.name currently hardcodes 'openai' in the registerInferenceProviderConnection call (see llmMetadata, baseURL, and registerInferenceProviderConnection) which can misrepresent non-OpenAI backends; update llmMetadata to either derive a more accurate identifier from baseURL (e.g., detect azure, local, proxy host patterns and set name accordingly) or use a neutral value such as 'openai-compatible' and include the endpoint in llmMetadata (e.g., endpoint/baseURL) so callers know this is the wire protocol not the upstream provider; change the llmMetadata object passed to registerInferenceProviderConnection to reflect this new naming strategy.
♻️ Duplicate comments (1)
packages/extension-api/src/extension-api.d.ts (1)
653-655:⚠️ Potential issue | 🟠 MajorKeep
LLMMetadataextensible.As previously flagged, the current shape only permits
name, which contradicts the PR discussion converging on a free-form metadata bag (e.g., addingversionand other LLM details). Callers wanting to attach anything beyondnamewill get type errors. Prefer an index signature (or at least declare known optional fields now) so this public API doesn't need another breaking revision shortly after shipping.🔧 Proposed shape
export interface LLMMetadata { name?: string; + version?: string; + [key: string]: string | undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-api/src/extension-api.d.ts` around lines 653 - 655, The LLMMetadata interface is too narrow (only name) and must be made extensible: update the exported interface LLMMetadata to allow arbitrary additional metadata (e.g., add an index signature like [key: string]: any or declare known optional fields such as version, provider, etc.) so callers can attach extra LLM details without type errors; modify the declaration of LLMMetadata in extension-api.d.ts accordingly while keeping name optional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@extensions/openai-compatible/src/openAI.ts`:
- Around line 187-205: The llmMetadata.name currently hardcodes 'openai' in the
registerInferenceProviderConnection call (see llmMetadata, baseURL, and
registerInferenceProviderConnection) which can misrepresent non-OpenAI backends;
update llmMetadata to either derive a more accurate identifier from baseURL
(e.g., detect azure, local, proxy host patterns and set name accordingly) or use
a neutral value such as 'openai-compatible' and include the endpoint in
llmMetadata (e.g., endpoint/baseURL) so callers know this is the wire protocol
not the upstream provider; change the llmMetadata object passed to
registerInferenceProviderConnection to reflect this new naming strategy.
---
Duplicate comments:
In `@packages/extension-api/src/extension-api.d.ts`:
- Around line 653-655: The LLMMetadata interface is too narrow (only name) and
must be made extensible: update the exported interface LLMMetadata to allow
arbitrary additional metadata (e.g., add an index signature like [key: string]:
any or declare known optional fields such as version, provider, etc.) so callers
can attach extra LLM details without type errors; modify the declaration of
LLMMetadata in extension-api.d.ts accordingly while keeping name optional.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5c3942ed-6774-4e29-8c1e-2b8efc8bf6e2
📒 Files selected for processing (14)
extensions/gemini/src/gemini.spec.tsextensions/gemini/src/gemini.tsextensions/mistral/src/manager/mistral-inference-manager.spec.tsextensions/mistral/src/manager/mistral-inference-manager.tsextensions/ollama/src/ollama-extension.tsextensions/openai-compatible/src/openAI.spec.tsextensions/openai-compatible/src/openAI.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/ramalama/src/manager/inference-model-manager.tspackages/api/src/provider-info.tspackages/extension-api/src/extension-api.d.tspackages/main/src/plugin/provider-registry.tspackages/renderer/src/lib/chat/components/model-info.tspackages/renderer/src/lib/models/models-utils.ts
📜 Review details
⏰ 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). (10)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: unit tests / macos-15
- GitHub Check: macOS
- GitHub Check: linter, formatters
- GitHub Check: unit tests / windows-2025
- GitHub Check: Linux
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: Windows
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases (e.g.,'/@/plugin/provider-registry.js') instead of relative paths (e.g.,'../plugin/provider-registry.js') for imports outside the current directory's module group. Relative imports are only used for sibling modules within the same directory.
Files:
extensions/gemini/src/gemini.tsextensions/ollama/src/ollama-extension.tsextensions/mistral/src/manager/mistral-inference-manager.tsextensions/mistral/src/manager/mistral-inference-manager.spec.tsextensions/openai-compatible/src/openAI.tsextensions/ramalama/src/manager/inference-model-manager.tspackages/main/src/plugin/provider-registry.tspackages/renderer/src/lib/models/models-utils.tspackages/api/src/provider-info.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/openai-compatible/src/openAI.spec.tspackages/renderer/src/lib/chat/components/model-info.tspackages/extension-api/src/extension-api.d.tsextensions/gemini/src/gemini.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in Vitest. Avoid manual mock factories (vi.mock('...', () => ({...}))) when possible
Usevi.resetAllMocks()inbeforeEachfor resetting mocks in Vitest unit tests, notvi.clearAllMocks()
When an auto-mocked function or class method needs a real implementation in Vitest, usevi.mocked(...). For class methods, use the prototype pattern:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
extensions/mistral/src/manager/mistral-inference-manager.spec.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/openai-compatible/src/openAI.spec.tsextensions/gemini/src/gemini.spec.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: Long-running operations should useTaskManagerwithcreateTask()to provide user feedback on operation status
Container operations in the main process must useContainerProviderRegistrywith theengineIdparameter to identify the container engine
Kubernetes operations in the main process should useKubernetesClientfor context management, resource operations, port forwarding, and exec operations
IPC handlers in the main process must follow the naming convention:<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
Store credentials and sensitive setup data securely viaSafeStorageRegistryinstead of plain configuration
Files:
packages/main/src/plugin/provider-registry.ts
packages/renderer/src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
External URLs in the renderer process require user confirmation (handled via
setupSecurityRestrictionsOnLinks)
Files:
packages/renderer/src/lib/models/models-utils.tspackages/renderer/src/lib/chat/components/model-info.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/models-utils.ts:11-20
Timestamp: 2026-04-17T20:26:32.946Z
Learning: In `packages/renderer/src/lib/models/models-utils.ts` (openkaiden/kaiden), `InferenceConnectionSummary.connectionType` is intentionally optional. It is only `undefined` for the single synthetic `'not-configured'` entry (emitted when a provider has `inferenceProviderConnectionCreation` but no active `inferenceConnections`). All consumers guard with optional chaining. A discriminated union was considered but deferred as unnecessary complexity for v1, since the invariant (`connectionType` is defined iff `status !== 'not-configured'`) is self-evident from the `status` field. Do not flag this as a type-safety issue.
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T04:24:07.577Z
Learning: Applies to extensions/*/package.json : Extensions should declare provider capabilities (inference providers, flow providers, MCP registries, configuration properties) in the `contributes` section of their `package.json`
📚 Learning: 2026-04-17T20:26:32.946Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/models-utils.ts:11-20
Timestamp: 2026-04-17T20:26:32.946Z
Learning: In `packages/renderer/src/lib/models/models-utils.ts` (openkaiden/kaiden), `InferenceConnectionSummary.connectionType` is intentionally optional. It is only `undefined` for the single synthetic `'not-configured'` entry (emitted when a provider has `inferenceProviderConnectionCreation` but no active `inferenceConnections`). All consumers guard with optional chaining. A discriminated union was considered but deferred as unnecessary complexity for v1, since the invariant (`connectionType` is defined iff `status !== 'not-configured'`) is self-evident from the `status` field. Do not flag this as a type-safety issue.
Applied to files:
extensions/gemini/src/gemini.tsextensions/mistral/src/manager/mistral-inference-manager.tsextensions/openai-compatible/src/openAI.tsextensions/ramalama/src/manager/inference-model-manager.tspackages/main/src/plugin/provider-registry.tspackages/renderer/src/lib/models/models-utils.tspackages/api/src/provider-info.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/openai-compatible/src/openAI.spec.tspackages/renderer/src/lib/chat/components/model-info.tspackages/extension-api/src/extension-api.d.tsextensions/gemini/src/gemini.spec.ts
📚 Learning: 2026-04-17T20:27:11.322Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/stores/model-catalog.ts:26-28
Timestamp: 2026-04-17T20:27:11.322Z
Learning: In `packages/renderer/src/stores/model-catalog.ts`, provider IDs used as the first component of `modelKey(providerId, label)` are always simple colon-free slug strings (e.g. `gemini`, `claude`, `openai`, `openshiftai`). The `:` separator in `modelKey` does not risk key collisions because provider IDs are guaranteed never to contain a colon by convention. Do not flag this as a collision risk in future reviews.
Applied to files:
extensions/ramalama/src/manager/inference-model-manager.tspackages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-16T04:24:07.577Z
Learnt from: CR
Repo: openkaiden/kaiden PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T04:24:07.577Z
Learning: Applies to extensions/*/package.json : Extensions should declare provider capabilities (inference providers, flow providers, MCP registries, configuration properties) in the `contributes` section of their `package.json`
Applied to files:
extensions/ramalama/src/manager/inference-model-manager.tspackages/extension-api/src/extension-api.d.ts
📚 Learning: 2026-04-20T14:30:15.867Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts:30-45
Timestamp: 2026-04-20T14:30:15.867Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.spec.ts`, the `stubOllama` helper intentionally only re-stubs `fetch` and relies on `beforeEach`'s `stubRamalama(false)` call to keep `getProviderInfos` in place. This is a deliberate standard Vitest `beforeEach` + per-test override pattern. Do not flag the implicit dependency between `stubOllama` and the `beforeEach` ramalama stub as a robustness issue.
Applied to files:
extensions/ramalama/src/manager/inference-model-manager.tsextensions/ramalama/src/manager/inference-model-manager.spec.tsextensions/gemini/src/gemini.spec.ts
📚 Learning: 2026-04-13T15:59:29.742Z
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1295
File: packages/extension-api/src/extension-api.d.ts:651-651
Timestamp: 2026-04-13T15:59:29.742Z
Learning: In the openkaiden/kaiden repository, the project relies on TypeScript's static type checking (not runtime validation) to enforce type correctness for `InferenceProviderConnection.type` (`InferenceProviderConnectionType`). Runtime normalization/validation guards for this field are not needed or desired.
Applied to files:
packages/main/src/plugin/provider-registry.tspackages/api/src/provider-info.tsextensions/openai-compatible/src/openAI.spec.tspackages/renderer/src/lib/chat/components/model-info.tspackages/extension-api/src/extension-api.d.ts
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/provider-registry.ts
📚 Learning: 2026-04-14T13:16:08.886Z
Learnt from: fbricon
Repo: openkaiden/kaiden PR: 1332
File: packages/renderer/src/stores/chat-window.ts:40-43
Timestamp: 2026-04-14T13:16:08.886Z
Learning: In `packages/renderer/src/stores/chat-window.ts` (openkaiden/kaiden), the `showChatWindow` store intentionally uses `showChatWindow.set(value === true)`, mapping `undefined` to `false`. The `chat.showChatWindow` setting has `default: false` in the schema (`packages/main/src/plugin/chat-init.ts`), making the chat window **opt-in** (hidden by default). Do not flag `value === true` as a bug — the chat should only be visible when the config value is explicitly `true`.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-17T20:26:55.521Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/ModelsCatalog.svelte:51-53
Timestamp: 2026-04-17T20:26:55.521Z
Learning: In `packages/renderer/src/lib/models/ModelsCatalog.svelte` (openkaiden/kaiden), `ModelSelectable = CatalogModelInfo & { selected: boolean }` is intentionally defined and `selected: false` is set on each filtered row. The `selected` field is structurally required by the `podman-desktop/ui-svelte` `Table` component's generic constraint (`T extends { selected?: boolean; name?: string }`). Removing it causes TypeScript type errors. Do not flag this as dead state.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-17T20:26:14.460Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/columns/ModelStatusColumn.svelte:15-24
Timestamp: 2026-04-17T20:26:14.460Z
Learning: In `packages/renderer/src/lib/models/columns/ModelStatusColumn.svelte`, the `statusMap` intentionally maps `unknown` → `'RUNNING'` because the Gemini extension reports `unknown` connection status even when the connection is healthy (API key accepted, models loaded). Flagging this as misleading is incorrect; changing it to `DEGRADED` would produce a false warning for healthy Gemini providers. The root fix (extensions should report `started`) is out of scope for UI-only PRs.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-22T02:58:56.754Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1379
File: packages/renderer/src/lib/models/ModelsCatalogEmptyScreen.spec.ts:32-38
Timestamp: 2026-04-22T02:58:56.754Z
Learning: In `openkaiden/kaiden`, test files under `packages/renderer/src/lib/models/**/*.spec.ts` (and sibling spec files) intentionally use exact user-facing copy strings in `screen.getByText(...)` assertions rather than regex matchers or `data-testid`. This is a deliberate Testing Library convention: if the wording changes, the test should fail to prompt an intentional update. Do not flag exact-string `getByText` assertions as brittle in these test files.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-21T09:42:09.739Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1396
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte:126-143
Timestamp: 2026-04-21T09:42:09.739Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte`, the UI copy referencing "the next step" (e.g., "Results update the default-model step." and "You can pick a default from the local catalog on the next step.") is intentionally forward-looking. A follow-up issue will add a model-selection step after the coding-agent step in the onboarding wizard. Do not flag these strings as referencing a non-existent step.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-23T11:33:39.165Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1431
File: packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte:43-52
Timestamp: 2026-04-23T11:33:39.165Z
Learning: In `packages/renderer/src/lib/guided-setup/CodingAgentStep.svelte` and `packages/renderer/src/lib/guided-setup/guided-setup-steps.ts` (openkaiden/kaiden), `CliAgent` is the source-of-truth union type representing agent names supported by the `kdn` CLI (e.g., used by `kdn init --agent`). The `agentDefinitions` registry in `agent-registry.ts` is always a UI-side subset that must conform to `CliAgent`, not define it. Do not suggest deriving `CliAgent` from the registry — that would invert the intended dependency direction. The `as CliAgent` cast in `CodingAgentStep.svelte` is safe by construction because `agentDefinitions[].cliName` is typed as `CliAgent`.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-22T09:55:01.642Z
Learnt from: fbricon
Repo: openkaiden/kaiden PR: 1418
File: packages/renderer/src/stores/navigation/navigation-registry-agent-workspaces.svelte.ts:25-28
Timestamp: 2026-04-22T09:55:01.642Z
Learning: In `packages/renderer/src/stores/navigation/navigation-registry-agent-workspaces.svelte.ts` (openkaiden/kaiden), the navigation sidebar label `name: 'Workspaces'` is intentionally short (not 'Agentic Workspaces'). This matches the approved design mockup. The full label is surfaced in the page title and tooltip ('Agentic Workspaces'). Do not flag the terse sidebar label as ambiguous or suggest expanding it to 'Agentic Workspaces'.
Applied to files:
packages/renderer/src/lib/models/models-utils.ts
📚 Learning: 2026-04-12T15:29:39.027Z
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1314
File: packages/extension-api/package.json:3-3
Timestamp: 2026-04-12T15:29:39.027Z
Learning: In `packages/extension-api/package.json` (openkaiden/kaiden), the `scripts.publish:next` field intentionally uses a hardcoded older prerelease base (e.g., `0.0.1-"$(date +%s)"`) even after the package `version` field is bumped (e.g., to `0.1.0-next`). This is by design: the project keeps older minor lines (like `0.0.x`) alive for backported fixes and security releases. Do not flag this as inconsistent with the current `version` field in future reviews.
Applied to files:
packages/extension-api/src/extension-api.d.ts
📚 Learning: 2026-04-12T15:29:06.175Z
Learnt from: jeffmaury
Repo: openkaiden/kaiden PR: 1314
File: extensions/container/packages/api/package.json:3-3
Timestamp: 2026-04-12T15:29:06.175Z
Learning: In `extensions/container/packages/api/package.json` (openkaiden/kaiden), the `scripts.publish:next` field intentionally uses a hardcoded older prerelease line (e.g., `0.0.1-"$(date +%s)"`) even after the package `version` field is bumped (e.g., to `0.1.0-next`). This is by design: the project keeps minor versions (like `0.0.1`) alive for backported fixes and security releases. Do not flag this as inconsistent with the current `version` field in future reviews.
Applied to files:
packages/extension-api/src/extension-api.d.ts
🔇 Additional comments (13)
packages/api/src/provider-info.ts (1)
22-22: LGTM.The optional
llmMetadatafield and theLLMMetadatatype import are correctly threaded intoProviderInferenceConnectionInfo. Keeping it optional avoids breaking existing consumers.Also applies to: 78-78
extensions/gemini/src/gemini.ts (1)
158-158: LGTM.
llmMetadata: { name: 'gemini' }is consistent with the convention used by the other inference extensions in this PR.extensions/ollama/src/ollama-extension.ts (1)
104-104: LGTM.Addition is consistent with the rest of the PR. No behavior change in the polling/re-registration flow.
extensions/mistral/src/manager/mistral-inference-manager.spec.ts (1)
164-164: LGTM.Assertion correctly mirrors the new
llmMetadatafield emitted by the implementation.extensions/mistral/src/manager/mistral-inference-manager.ts (1)
111-111: LGTM.Matches the pattern adopted by the other providers and is covered by the corresponding spec update.
packages/extension-api/src/extension-api.d.ts (1)
657-668: Claude extension does not includellmMetadatain connection registration.While Gemini, Mistral, Ollama, OpenAI-compatible, and Ramalama all register with
llmMetadata: { name: 'provider-name' }, the Claude extension registers without this field. SincellmMetadatais optional in the type, this compiles without error. Verify whether this omission is intentional or an oversight inextensions/claude/src/manager/claude-inference-manager.ts(line 108).extensions/ramalama/src/manager/inference-model-manager.ts (1)
58-71: Inconsistency:llmMetadata.nameis'openai'but the provider id is'ramalama'.Other extensions set
llmMetadata.nameto match theirprovider.id(ollama,mistral,gemini). Here, RamaLama advertises'openai', which conflates the runtime (RamaLama) with the wire protocol (OpenAI-compatible). If the intent is to convey "OpenAI-compatible API surface" to agents, consider making that explicit (e.g., a separateapiCompatibilityfield orname: 'ramalama'with a note about the wire format) so downstream consumers can distinguish the hosting provider from the protocol.Given the recent API discussion converged on
llmMetadatabeing a free-form bag, this may be intentional — please confirm which semantic (provider identity vs. protocol compatibility)llmMetadata.nameis meant to carry, and document it on theLLMMetadatatype.What field name conventions do LLM agent frameworks (e.g., LangChain, LlamaIndex, Vercel AI SDK) use to distinguish a provider's identity from its wire-protocol/API compatibility?packages/main/src/plugin/provider-registry.ts (1)
790-799: LGTM!The
llmMetadatapass-through aligns with the new optional field onProviderInferenceConnectionInfoandInferenceProviderConnection. Preservingundefined(rather than defaulting) is correct since the field is optional on both ends.extensions/gemini/src/gemini.spec.ts (1)
197-208: LGTM!Assertion correctly mirrors the updated registration payload with
llmMetadata: { name: 'gemini' }, matching theprovider.id'gemini'.extensions/ramalama/src/manager/inference-model-manager.spec.ts (1)
69-78: LGTM!Test assertion mirrors the production registration payload. If the production
llmMetadata.namevalue is changed per the concern raised ininference-model-manager.ts, this expectation will need to be updated in lockstep.packages/renderer/src/lib/models/models-utils.ts (1)
4-25: LGTM!
llmMetadatais correctly destructured from each inference connection and forwarded intoModelInfo. Since both source and target fields are optional,undefinedpasses through safely.packages/renderer/src/lib/chat/components/model-info.ts (1)
19-28: LGTM!Optional
llmMetadataaddition toModelInfois backward-compatible and matches the shared type imported from@openkaiden/api.extensions/openai-compatible/src/openAI.spec.ts (1)
177-188: LGTM!New assertion correctly verifies the
llmMetadatapayload added to the registration call.
Fixes openkaiden#1425 Signed-off-by: Jeff MAURY <jmaury@redhat.com>
Signed-off-by: Jeff MAURY <jmaury@redhat.com>
Signed-off-by: Jeff MAURY <jmaury@redhat.com>
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
3 participants
Fixes #1425